Signal handling/zcurses

Signal handling/zcurses

[Anthony Charles]

Hi guys,

I have a script which loops over zcurses input to get user input and I
set a trap on USR1 to launch a function.

If I kill -USR1 this script, the function is executed as planned but
then zcurses input terminated and ends the loop.

Why does zcurses terminated on USR1 ? Is there a way to change that
apart of changing module source code (or maybe this is a bug) ?

Thanks.
-- 
Anthony CHARLES

Re: Signal handling/zcurses

[Bart Schaefer]

On Apr 20, 12:19pm, Anthony Charles wrote:
}
} Why does zcurses terminated on USR1 ? Is there a way to change that
} apart of changing module source code (or maybe this is a bug) ?

It's difficult to say without seeing the actual script (or at least the
part of it where you loop to read input), but it's very possible that
it's related to this:

2011-04-11  Peter Stephenson  <pws@csr.com>

        * users/15953: Src/builtin.c: handle EINTR when using read -k or
        -q together with -u or -p.

This only affects operating systems wherein system calls are not
automatically restarted after a signal.  It's also possible that it's
not fixed by that patch, rather that there's yet another place where
EINTR has to be handled specially.

Re: Signal handling/zcurses

[Anthony Charles]

Just tested with zsh from cvs including the patch 
you mentioned and it does not work :

 % echo $ZSH_VERSION $ZSH_PATCHLEVEL
 4.3.11-dev-2 1.5254

About the loop, it's as follow :
 while zcurses input $main_screen inp spe_inp
 do
	...
 done

-- 
Anthony CHARLES

Re: Signal handling/zcurses

[Bart Schaefer]

On Apr 20,  9:22pm, Anthony Charles wrote:
}
} About the loop, it's as follow :
}  while zcurses input $main_screen inp spe_inp
}  do
} 	...
}  done

Looking at Src/Modules/curses.c ... this is implmented using one of
wget_wch() or wgetch() depending on the version of libcurses that is
linked.

So in all likelihood it's somewhere in libcurses that this is getting
interrupted, and it's unclear whether there's any way for the caller
to determine why that call failed and thereby decide to retry it.

Re: Signal handling/zcurses

[Anthony Charles]

Thanks for the info. Even SIGWINCH breaks the loop and terminate my
script, which is quite annoying.

man 3 getch says about wgetch in portability section that it may not
be interrupted by signals or it may return ERR with errno set to EINTR
depending of the implementation and OS. In my case, on Debian it
seems it's the second choice :)

-- 
Anthony CHARLES

Re: Signal handling/zcurses

[Bart Schaefer]

On Apr 21,  4:31pm, Anthony Charles wrote:
} 
} man 3 getch says about wgetch in portability section that it may not
} be interrupted by signals or it may return ERR with errno set to EINTR
} depending of the implementation and OS. In my case, on Debian it
} seems it's the second choice :)

Try this.

Index: Src/Modules/curses.c
===================================================================
--- curses.c	4 Nov 2008 04:47:53 -0000	1.4
+++ curses.c	21 Apr 2011 15:39:05 -0000
@@ -1070,7 +1070,11 @@
 #endif
 
 #ifdef HAVE_WGET_WCH
-    switch (wget_wch(w->win, &wi)) {
+    while ((errno = 0), (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 +1096,10 @@
 	return 1;
     }
 #else
-    ci = wgetch(w->win);
-    if (ci == ERR)
-	return 1;
+    while ((errno = 0), (ci = wgetch(w->win)) == ERR) {
+	if (errno != EINTR)
+	    return 1;
+    }
     if (ci >= 256) {
 	keypadnum = ci;
 	*instr = '\0';


-- 

Re: Signal handling/zcurses

[Anthony Charles]

Unforunately, I see no changes with this patch but with little debug, I
found out that errno is first set to 4 (EINTR), then wgetch is restarted but
failed, errno is set to 0 then my shell while loop breaks and the
script terminate.
Here's a trace :

[...]
 # refresh_screen is the function tied to USR1
+refresh_screen:28> zcurses move zmpc 0 0
 # errno from the while loop in curses.c
wgetch -- errno : 4
wgetch -- errno : 0
 # end of script, cleaning zcurses
+zmpc.sh:250> zcurses delwin zmpc
+zmpc.sh:251> zcurses delwin zmpc_status
+zmpc.sh:252> zcurses delwin zmpc_currentsong
+zmpc.sh:253> zcurses end
+zmpc.sh:257> exit 0

Strangely, if I comment my trap and kill -USR1 the script, it
terminates immediately, no debug from curses.c as if it didn't receive
the signal.


On Thu, Apr 21, 2011 at 08:47:15AM -0700, Bart Schaefer wrote:
> On Apr 21,  4:31pm, Anthony Charles wrote:
> } 
> } man 3 getch says about wgetch in portability section that it may not
> } be interrupted by signals or it may return ERR with errno set to EINTR
> } depending of the implementation and OS. In my case, on Debian it
> } seems it's the second choice :)
> 
> Try this.
> 
> Index: Src/Modules/curses.c
> ===================================================================
> --- curses.c	4 Nov 2008 04:47:53 -0000	1.4
> +++ curses.c	21 Apr 2011 15:39:05 -0000
> @@ -1070,7 +1070,11 @@
>  #endif
>  
>  #ifdef HAVE_WGET_WCH
> -    switch (wget_wch(w->win, &wi)) {
> +    while ((errno = 0), (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 +1096,10 @@
>  	return 1;
>      }
>  #else
> -    ci = wgetch(w->win);
> -    if (ci == ERR)
> -	return 1;
> +    while ((errno = 0), (ci = wgetch(w->win)) == ERR) {
> +	if (errno != EINTR)
> +	    return 1;
> +    }
>      if (ci >= 256) {
>  	keypadnum = ci;
>  	*instr = '\0';
> 
> 
> -- 

-- 
Anthony CHARLES

Re: Signal handling/zcurses

[Anthony Charles]

[-- Attachment #1: Type: text/plain, Size: 68 bytes --]

Here is a minimal script to test this problem.

-- 
Anthony CHARLES

[-- Attachment #2: test_zsh_curses.sh --]
[-- Type: application/x-sh, Size: 305 bytes --]

Re: Signal handling/zcurses

[Bart Schaefer]

On Apr 21,  8:08pm, Anthony Charles wrote:
}
} Unforunately, I see no changes with this patch but with little debug,
} I found out that errno is first set to 4 (EINTR), then wgetch is
} restarted but failed

That would seem to indicate that once the read system call has been
interrupted inside wgetch(), it's not possible to pick up where you
left off by simply calling wgetch() again.

Interestingly, I'm able to reproduce your result with the script you
sent -- in spite of the fact that (a) Linux normally has restartable
system calls (b) the doc says that "Under the ncurses implementation,
handled signals never interrupt getch" and (c) my zsh is linked with
-lncursesw so this really should work.

An strace says

read(0, 0xbff8d41f, 1)                  = ? ERESTARTSYS (To be restarted)
--- SIGUSR1 (User defined signal 1) @ 0 (0) ---
rt_sigprocmask(SIG_BLOCK, ~[RTMIN RT_1], [USR1 CHLD], 8) = 0
rt_sigprocmask(SIG_SETMASK, [USR1 CHLD], ~[KILL STOP RTMIN RT_1], 8) = 0
rt_sigprocmask(SIG_BLOCK, [CHLD], [USR1 CHLD], 8) = 0
write(1, "USR1\n", 5)                   = 5
rt_sigprocmask(SIG_BLOCK, [CHLD], [USR1 CHLD], 8) = 0
rt_sigprocmask(SIG_UNBLOCK, [CHLD], [USR1 CHLD], 8) = 0
sigreturn()                             = ? (mask now [CHLD])
rt_sigprocmask(SIG_BLOCK, [CHLD], [CHLD], 8) = 0

This is strange because at this point [after sigreturn()] the read()
call should have been resumed, but that never happens.  GDB confirms
that the loop in my patch does call wget_wch() again, so EINTR was
returned (which surprises me, see above) but that wget_wch() returns
without making any system calls (which I think means it believes the
window to be invalid, but I'm not really sure).

Everything works (and straces) as expected with STOP/CONT signals.

read(0, 0xbfe8360f, 1)                  = ? ERESTARTSYS (To be restarted)
--- SIGSTOP (Stopped (signal)) @ 0 (0) ---
--- SIGSTOP (Stopped (signal)) @ 0 (0) ---
read(0, 0xbfe8360f, 1)                  = ? ERESTARTSYS (To be restarted)
--- SIGCONT (Continued) @ 0 (0) ---
read(0,


} Strangely, if I comment my trap and kill -USR1 the script, it
} terminates immediately, no debug from curses.c as if it didn't receive
} the signal.

That's not strange at all -- the default response to USR1 is for the OS
to kill the process.  If you haven't trapped it, then zsh has not changed
that default, so ...

Re: Signal handling/zcurses

[Bart Schaefer]

On Apr 21,  8:07pm, Bart Schaefer wrote:
}
} Interestingly, I'm able to reproduce your result with the script you
} sent -- in spite of the fact that (a) Linux normally has restartable
} system calls (b) the doc says that "Under the ncurses implementation,
} handled signals never interrupt getch" and (c) my zsh is linked with
} -lncursesw so this really should work.

This gets odder.

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.

Run "zcurses input" again immediately, and it again gets ERR/zero
without ever calling read().

Run it *again* and not only does wget_wch() block waiting for input,
but now it restarts properly after a handled signal!  Give it some
input so it returns success and the script-level loop in the test goes
around and wget_wch() is called a fifth time, and now it has returned
to the state where it gets interrupted by handled signals.

I was trying all this inside of GDB so I suppose that might have side-
effects, but this is damned peculiar.

Re: Signal handling/zcurses

[Bart Schaefer]

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';

Re: Signal handling/zcurses

[Anthony Charles]

This patch works for me, thanks Bart.

-- 
Anthony CHARLES