Cope with Readline's failure to track SIGWINCH events outside of input.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 16 Dec 2015 21:58:56 +0000 (16:58 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 16 Dec 2015 21:58:56 +0000 (16:58 -0500)
It emerges that libreadline doesn't notice terminal window size change
events unless they occur while collecting input.  This is easy to stumble
over if you resize the window while using a pager to look at query output,
but it can be demonstrated without any pager involvement.  The symptom is
that queries exceeding one line are misdisplayed during subsequent input
cycles, because libreadline has the wrong idea of the screen dimensions.

The safest, simplest way to fix this is to call rl_reset_screen_size()
just before calling readline().  That causes an extra ioctl(TIOCGWINSZ)
for every command; but since it only happens when reading from a tty, the
performance impact should be negligible.  A more valid objection is that
this still leaves a tiny window during entry to readline() wherein delivery
of SIGWINCH will be missed; but the practical consequences of that are
probably negligible.  In any case, there doesn't seem to be any good way to
avoid the race, since readline exposes no functions that seem safe to call
from a generic signal handler --- rl_reset_screen_size() certainly isn't.

It turns out that we also need an explicit rl_initialize() call, else
rl_reset_screen_size() dumps core when called before the first readline()
call.

rl_reset_screen_size() is not present in old versions of libreadline,
so we need a configure test for that.  (rl_initialize() is present at
least back to readline 4.0, so we won't bother with a test for it.)
We would need a configure test anyway since libedit's emulation of
libreadline doesn't currently include such a function.  Fortunately,
libedit seems not to have any corresponding bug.

Merlin Moncure, adjusted a bit by me

configure
configure.in
src/bin/psql/input.c
src/include/pg_config.h.in

index 1f6d7933f9c73ad3ea67ce91eb5fae8b926ccf86..0b6c5752b7b6bbbd9168fa9e3371c93db1494f77 100755 (executable)
--- a/configure
+++ b/configure
@@ -23217,7 +23217,8 @@ _ACEOF
 fi
 
 
-for ac_func in rl_completion_matches rl_filename_completion_function
+
+for ac_func in rl_completion_matches rl_filename_completion_function rl_reset_screen_size
 do
 as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 { $as_echo "$as_me:$LINENO: checking for $ac_func" >&5
index 9671a8d214cdf035ff3a5ae4daae9aa771ec2b07..43a10512b372dccdcbae2f5e7c08f6979cf01d89 100644 (file)
@@ -1508,7 +1508,7 @@ LIBS="$LIBS_including_readline"
 
 if test "$with_readline" = yes; then
   PGAC_VAR_RL_COMPLETION_APPEND_CHARACTER
-  AC_CHECK_FUNCS([rl_completion_matches rl_filename_completion_function])
+  AC_CHECK_FUNCS([rl_completion_matches rl_filename_completion_function rl_reset_screen_size])
   AC_CHECK_FUNCS([append_history history_truncate_file])
 fi
 
index f08ed83d68ef99a17dd71f912bf67f457c6df5e6..1e5bba908a75614392c74c3c18b9a3094f3faa8e 100644 (file)
@@ -65,6 +65,17 @@ gets_interactive(const char *prompt)
    {
        char       *result;
 
+       /*
+        * Some versions of readline don't notice SIGWINCH signals that arrive
+        * when not actively reading input.  The simplest fix is to always
+        * re-read the terminal size.  This leaves a window for SIGWINCH to be
+        * missed between here and where readline() enables libreadline's
+        * signal handler, but that's probably short enough to be ignored.
+        */
+#ifdef HAVE_RL_RESET_SCREEN_SIZE
+       rl_reset_screen_size();
+#endif
+
        /* Enable SIGINT to longjmp to sigint_interrupt_jmp */
        sigint_interrupt_enabled = true;
 
@@ -330,6 +341,7 @@ initializeInput(int flags)
        char        home[MAXPGPATH];
 
        useReadline = true;
+       rl_initialize();
        initialize_readline();
 
        useHistory = true;
index b6a6a48807e47db24941bfca4d928ee122ec0639..1c49fcb1225bf12d6d28d12f321b04431a0a8b20 100644 (file)
 /* Define to 1 if you have the `rl_filename_completion_function' function. */
 #undef HAVE_RL_FILENAME_COMPLETION_FUNCTION
 
+/* Define to 1 if you have the `rl_reset_screen_size' function. */
+#undef HAVE_RL_RESET_SCREEN_SIZE
+
 /* Define to 1 if you have the <security/pam_appl.h> header file. */
 #undef HAVE_SECURITY_PAM_APPL_H