Re: Signal handling/zcurses

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Apr 21, 10:52pm, Bart Schaefer wrote:
} Subject: Re: Signal handling/zcurses
}
} If you interrupt "zcurses input" with a handled signal, wget_wch() [and
} I must assume wgetch()] returns ERR/EINTR.  The loop in my patch then
} calls it again and gets ERR/zero, which causes "zcurses input" to
} return 1.

Let's try this patch instead.

Note - this change may still be incomplete.  When the "read" builtin is
interrupted its implementation checks various global shell conditions in
addition to the errno state.  It's not clear to me which of those should
be checked here.


Index: Src/Modules/curses.c
===================================================================
RCS file: /extra/cvsroot/zsh/zsh-4.0/Src/Modules/curses.c,v
retrieving revision 1.4
diff -c -r1.4 curses.c
--- curses.c	4 Nov 2008 04:47:53 -0000	1.4
+++ curses.c	22 Apr 2011 14:12:41 -0000
@@ -1069,8 +1069,47 @@
     }
 #endif
 
+    /*
+     * Some documentation for wgetch() says:
+
+       The behavior of getch and friends in the presence of  handled  signals
+       is  unspecified  in the SVr4 and XSI Curses documentation.  Under his-
+       torical curses implementations, it varied  depending  on  whether  the
+       operating system's implementation of handled signal receipt interrupts
+       a read(2) call in progress or not, and also (in some  implementations)
+       depending  on  whether  an input timeout or non-blocking mode has been
+       set.
+
+       Programmers concerned about portability should be prepared for  either
+       of  two cases: (a) signal receipt does not interrupt getch; (b) signal
+       receipt interrupts getch and causes it to return ERR with errno set to
+       EINTR.  Under the ncurses implementation, handled signals never inter-
+       rupt getch.
+
+     * The observed behavior, however, is different:  wgetch() consistently
+     * returns ERR with EINTR when a signal is handled by the shell "trap"
+     * command mechanism.  Further, it consistently returns ERR twice, the
+     * second time without even attempting to repeat the interrupted read,
+     * which has the side-effect of NOT updating errno.  A third call will
+     * then begin reading again.
+     *
+     * Therefore, to properly implement signal trapping, we must (1) call
+     * wgetch() in a loop as long as errno remains EINTR, and (2) clear
+     * errno only before beginning the loop, not on every pass.
+     *
+     * There remains a potential bug here in that, if the caller has set
+     * a timeout for the read [see zccmd_timeout()] the countdown is very
+     * likely restarted on every call to wgetch(), so an interrupted call
+     * might wait much longer than desired.
+     */
+    errno = 0;
+
 #ifdef HAVE_WGET_WCH
-    switch (wget_wch(w->win, &wi)) {
+    while ((ret = wget_wch(w->win, &wi)) == ERR) {
+	if (errno != EINTR)
+	    break;
+    }
+    switch (ret) {
     case OK:
 	ret = wctomb(instr, (wchar_t)wi);
 	if (ret == 0) {
@@ -1092,9 +1131,10 @@
 	return 1;
     }
 #else
-    ci = wgetch(w->win);
-    if (ci == ERR)
-	return 1;
+    while ((ci = wgetch(w->win)) == ERR) {
+	if (errno != EINTR)
+	    return 1;
+    }
     if (ci >= 256) {
 	keypadnum = ci;
 	*instr = '\0';