[PATCH] Asynchronously resize the TUI

[PATCH] Asynchronously resize the TUI

[Patrick Palka]

This patch teaches the TUI to resize itself asynchronously instead of
synchronously.  Asynchronously resizing the screen when the underlying
terminal gets resized is the more intuitive behavior and is surprisingly
simple to implement thanks to GDB's async infrastructure.

The implementation is straightforward.  TUI's SIGWINCH handler is just
tweaked to asynchronously invoke a new callback,
tui_async_resize_screen, which is responsible for safely resizing the
screen.  Care must be taken to not to attempt to asynchronously resize
the screen while the TUI is not active.  When the TUI is not active, the
callback will do nothing, but the screen will yet be resized in the next
call to tui_enable() by virtue of win_resized being TRUE.

(So, after the patch there are still two places where the screen gets
resized: one in tui_enable() and the other now in
tui_async_resize_screen() as opposed to being in
tui_handle_resize_during_io().  The one in tui_enable() is still
necessary to handle the case where the terminal gets resized inside the
CLI: in that case, the TUI still needs resizing, but it must wait until
the TUI gets re-enabled.)

gdb/ChangeLog:

	* tui/tui-io.c (tui_handle_resize_during_io): Remove this
	function.
	(tui_putc): Don't call tui_handle_resize_during_io.
	(tui_getc): Likewise.
	(tui_mld_getc): Likewise.
	* tui/tui-win.c: Include event-loop.h and tui/tui-io.h.
	(tui_sigwinch_token): New static variable.
	(tui_initialize_win): Adjust documentation.  Set
	tui_sigwinch_token.
	(tui_async_resize_screen): New asynchronous callback.
	(tui_sigwinch_handler): Adjust documentation.  Asynchronously
	invoke tui_async_resize_screen.
---
 gdb/tui/tui-io.c  | 27 ---------------------------
 gdb/tui/tui-win.c | 53 +++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 41 insertions(+), 39 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 12bd29f..a8af9b6 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -136,8 +136,6 @@ static int tui_readline_pipe[2];
    This may be the main gdb prompt or a secondary prompt.  */
 static char *tui_rl_saved_prompt;
 
-static int tui_handle_resize_during_io (int, int);
-
 static void
 tui_putc (char c)
 {
@@ -397,8 +395,6 @@ tui_mld_getc (FILE *fp)
   WINDOW *w = TUI_CMD_WIN->generic.handle;
   int c = wgetch (w);
 
-  c = tui_handle_resize_during_io (c, 1);
-
   return c;
 }
 
@@ -593,7 +589,6 @@ tui_getc (FILE *fp)
 #endif
 
   ch = wgetch (w);
-  ch = tui_handle_resize_during_io (ch, 0);
 
   /* The \n must be echoed because it will not be printed by
      readline.  */
@@ -719,25 +714,3 @@ tui_expand_tabs (const char *string, int col)
 
   return ret;
 }
-
-/* Cleanup when a resize has occured.
-   Returns the character that must be processed.  */
-
-static int
-tui_handle_resize_during_io (int original_ch, int for_completion)
-{
-  if (tui_win_resized ())
-    {
-      tui_resize_all ();
-      tui_refresh_all_win ();
-      tui_update_gdb_sizes ();
-      tui_set_win_resized_to (FALSE);
-      if (!for_completion)
-	{
-	  dont_repeat ();
-	  return '\n';
-	}
-    }
-
-  return original_ch;
-}
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 1a12cc6..0a15030 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -32,8 +32,10 @@
 #include "cli/cli-cmds.h"
 #include "top.h"
 #include "source.h"
+#include "event-loop.h"
 
 #include "tui/tui.h"
+#include "tui/tui-io.h"
 #include "tui/tui-data.h"
 #include "tui/tui-wingeneral.h"
 #include "tui/tui-stack.h"
@@ -829,36 +831,63 @@ tui_resize_all (void)
 }
 
 #ifdef SIGWINCH
-/* SIGWINCH signal handler for the tui.  This signal handler is always
-   called, even when the readline package clears signals because it is
-   set as the old_sigwinch() (TUI only).  */
+/* Token for use by TUI's asynchronous SIGWINCH handler.  */
+static struct async_signal_handler *tui_sigwinch_token;
+
+/* TUI's SIGWINCH signal handler.  */
 static void
 tui_sigwinch_handler (int signal)
 {
-  /* Say that a resize was done so that the readline can do it later
-     when appropriate.  */
+  /* Set win_resized to TRUE and asynchronously invoke our resize callback.  If
+     the callback is invoked while TUI is active then it ought to successfully
+     resize the screen, resetting win_resized to FALSE.  Of course, if the
+     callback is invoked while TUI is inactive then it will do nothing; in that
+     case, win_resized will remain TRUE until we get a chance to synchronously
+     resize the screen from tui_enable().  */
+  mark_async_signal_handler (tui_sigwinch_token);
   tui_set_win_resized_to (TRUE);
 }
+
+/* Callback for asynchronously resizing TUI following a SIGWINCH signal.  */
+static void
+tui_async_resize_screen (gdb_client_data arg)
+{
+  if (!tui_active)
+    return;
+
+  tui_resize_all ();
+  tui_refresh_all_win ();
+  tui_update_gdb_sizes ();
+  tui_set_win_resized_to (FALSE);
+  tui_redisplay_readline ();
+}
 #endif
 
-/* Initializes SIGWINCH signal handler for the tui.  */
+/* Initialize TUI's SIGWINCH signal handler.  Note that the handler is not
+   uninstalled when we exit TUI, so the handler should not assume that TUI is
+   always active.  */
 void
 tui_initialize_win (void)
 {
 #ifdef SIGWINCH
+  tui_sigwinch_token
+    = create_async_signal_handler (tui_async_resize_screen, NULL);
+
+  {
 #ifdef HAVE_SIGACTION
-  struct sigaction old_winch;
+    struct sigaction old_winch;
 
-  memset (&old_winch, 0, sizeof (old_winch));
-  old_winch.sa_handler = &tui_sigwinch_handler;
+    memset (&old_winch, 0, sizeof (old_winch));
+    old_winch.sa_handler = &tui_sigwinch_handler;
 #ifdef SA_RESTART
-  old_winch.sa_flags = SA_RESTART;
+    old_winch.sa_flags = SA_RESTART;
 #endif
-  sigaction (SIGWINCH, &old_winch, NULL);
+    sigaction (SIGWINCH, &old_winch, NULL);
 #else
-  signal (SIGWINCH, &tui_sigwinch_handler);
+    signal (SIGWINCH, &tui_sigwinch_handler);
 #endif
 #endif
+  }
 }
 
 
-- 
2.3.0.80.g18d0fec

Re: [PATCH] Asynchronously resize the TUI

[Pedro Alves]

On 02/18/2015 12:36 AM, Patrick Palka wrote:

> gdb/ChangeLog:
> 
> 	* tui/tui-io.c (tui_handle_resize_during_io): Remove this
> 	function.
> 	(tui_putc): Don't call tui_handle_resize_during_io.
> 	(tui_getc): Likewise.
> 	(tui_mld_getc): Likewise.
> 	* tui/tui-win.c: Include event-loop.h and tui/tui-io.h.
> 	(tui_sigwinch_token): New static variable.
> 	(tui_initialize_win): Adjust documentation.  Set
> 	tui_sigwinch_token.
> 	(tui_async_resize_screen): New asynchronous callback.
> 	(tui_sigwinch_handler): Adjust documentation.  Asynchronously
> 	invoke tui_async_resize_screen.


OK.  Thanks for doing this!

Thanks,
Pedro Alves

Re: [PATCH] Asynchronously resize the TUI

[Patrick Palka]

On Wed, Feb 18, 2015 at 4:59 AM, Pedro Alves <palves@redhat.com> wrote:
> On 02/18/2015 12:36 AM, Patrick Palka wrote:
>
>> gdb/ChangeLog:
>>
>>       * tui/tui-io.c (tui_handle_resize_during_io): Remove this
>>       function.
>>       (tui_putc): Don't call tui_handle_resize_during_io.
>>       (tui_getc): Likewise.
>>       (tui_mld_getc): Likewise.
>>       * tui/tui-win.c: Include event-loop.h and tui/tui-io.h.
>>       (tui_sigwinch_token): New static variable.
>>       (tui_initialize_win): Adjust documentation.  Set
>>       tui_sigwinch_token.
>>       (tui_async_resize_screen): New asynchronous callback.
>>       (tui_sigwinch_handler): Adjust documentation.  Asynchronously
>>       invoke tui_async_resize_screen.
>
>
> OK.  Thanks for doing this!

Thanks for reviewing.  I have committed the patch in two pieces
because I forgot to add the gdb/ChangeLog entry in the first commit.
Sorry about that...

BTW shouldn't the object returned by create_async_signal_handler() be
eventually deallocated via delete_async_event_handler() at some point?
 It doesn't seem that we do this for any async signal handler
currently in use so far..

>
> Thanks,
> Pedro Alves
>

Re: [PATCH] Asynchronously resize the TUI

[Pedro Alves]

On 02/19/2015 12:03 AM, Patrick Palka wrote:

> Thanks for reviewing.  I have committed the patch in two pieces
> because I forgot to add the gdb/ChangeLog entry in the first commit.
> Sorry about that...

No worries.

> BTW shouldn't the object returned by create_async_signal_handler() be
> eventually deallocated via delete_async_event_handler() at some point?
>  It doesn't seem that we do this for any async signal handler
> currently in use so far..

Yeah, that's not really different from all the global objects that
are allocated on the heap, and aren't ever deleted.  They'll just go away
when GDB exits along with the heap.  For a proper GDB-as-a-library use
case though, we'd/we'll probably need to consider this.

Thanks,
Pedro Alves