Re: [PATCH] Explicitly call rl_resize_terminal() in TUI's SIGWINCH handler

[Pedro Alves <palves at redhat dot com>]

On 04/23/2015 12:49 AM, Patrick Palka wrote:
> (I am looking into syncing our copy of readline to the latest version,
> 6.3.)
> 
> In readline 6.3, the semantics of SIGWINCH handling has changed.
> When a SIGWINCH signal is raised, readline's rl_sigwinch_handler() now
> does not immediately call rl_resize_terminal().  Instead it sets a flag
> that is checked by RL_CHECK_SIGNALS() at a point where readline has
> control, and calls rl_resize_terminal() if said flag is set.
> 
> This change is item (c) in https://cnswww.cns.cwru.edu/php/chet/readline/CHANGES
> 
>   c.  Fixed a bug that caused readline to try and run code to modify its idea
>       of the screen size in a signal handler context upon receiving a SIGWINCH.
> 
> This change in behavior is important to us because TUI's
> tui_sigwinch_handler() relies on the assumption that by the time it's
> called, readline will have updated its knowledge of the terminal
> dimensions via rl_resize_terminal().  Since this assumption no longer
> holds true, TUI's SIGWINCH handling does not work correctly with
> readline 6.3.
> 
> To fix this issue this patch makes TUI explicitly call
> rl_resize_terminal() in tui_async_resize_screen() at the point where
> current terminal dimensions are needed.  (We could call it in
> tui_sigwinch_handler too, but since readline avoids doing it, we are
> probably safer off avoiding to call it in signal handler context as
> well.)  After this change, SIGWINCH handling continues to work properly
> with both readline 6.2 and 6.3.

OK with ChangeLog entry.  I wonder if this fixes PR18155?

I think we should call this and update gdb's sense of the screen
size even with the tui disabled though.  I currently see, outside
the TUI:

(gdb) show height
Number of lines gdb thinks are in a page is 45.

*resize*

(gdb) show height
Number of lines gdb thinks are in a page is 45.

* switch TUI on and off *

(gdb) show height
Number of lines gdb thinks are in a page is 27.

> 
> Since we no longer need it, we could now explicitly disable readline's
> SIGWINCH handler by setting rl_handle_sigwinch to zero early on in the
> program startup but I can't seem to find a good spot to place this
> assignment (the first call to rl_initialize() occurs in
> tui_initialize_readline() so the assignment should occur before then),
> and the handler is harmless anyway.

ITYM rl_catch_sigwinch:

/* If non-zero, readline will install a signal handler for SIGWINCH
   that also attempts to call any calling application's SIGWINCH signal
   handler.  Note that the terminal is not cleaned up before the
   application's signal handler is called; use rl_cleanup_after_signal()
   to do that. */
extern int rl_catch_sigwinch;

Maybe in init_page_info, and probably move the init_page_info call
to gdb_init directly for clarity.  (I found that with "rbreak rl_*".)

Thanks,
Pedro Alves