Re: lynx-dev lynx and ncurses and gpm

[Alexander V. Lukyanov]

On Thu, Apr 22, 1999 at 07:37:37AM -0500, Klaus Weide wrote:
> Problem 1a
> ----------
>      Suspending and resuming twice in a row, without any intervening
>      keystrokes - that is:  ^Z, fg, ^Z, fg
>      After the second fg, linux exits abruptly.
>      Does not happen if any keys are pressed between first fg and
>      second ^Z.
> 
> Reason: gpm messes around with interrupt handlers.  It installs a
> SIGTSTP handler (which in turn invokes the existing, i.e. ncurses
> handler), uses sigaction() for this and doesn't set a SA_RESTART flag.
> As a result SIGTSTP [as well as probably SIGWINCH, there is a similar 
> situation] is non-restarting, although other signals may be restarting.
> 
> The ncurses wgetch(), actually fifo_push() in lib_getch.c, doesn't deal
> with this situation, an interrupted wgetch() returns ERR, lynx concludes
> there is a severe error and exits.

>  This is consistent with the comments
> at the beginning of ncurses' lib_tstp.c:
> 
>  * The ncurses code needs signal-call restart to happen -- otherwise,
>  * interrupted wgetch() calls will return FAIL, probably making the
>  * application think the input stream has ended and it should
>  * terminate.
> 
> There is a provision to deal with an interrupted read() in fifo_push()
> if HIDE_EINTR is defined.  But HIDE_EINTR is explicitly #undef'd in
> fifo_defs.h, with no explanation except the 980606 NEWS file entry
> (which I assume corresponds to the #undef)
> 
>         + removed the cycle on EINTR, as it seems to be useless.
> 
> I don't know how someone came to that conclusion, and disagree with
> it (as does the lib_tstp.c comment above).

It is done for purpose. E.g. in case of xterm resize application should
act immediately, and EINTR for SIGWINCH helps that. I'm sure applications
can want to react to other signals outside of signal handlers (to avoid
reentering non-reenterable functions) by setting a flag inside and then
checking it in main loop.

getch can also return ERR on timeout if one was set.

So I beleave applications should not immediately exit on ERR from getch.

What is really bad in getch implementation is lack of distinction
between ERR as retriable error (such as EINTR or EAGAIN) and hard error
like terminal hangup.

> Possible solutions:
> 
> 1. Change gpm code to set SA_RESTART flag.
> 
>    Should be done, but this solves the problem only for ncurses+gpm, it
>    will pop up again for ncurses+<some other code out of ncurses' control>. 

IMHO this is simplest solution without side effects.

> 2. Change ncurses code to prevent gpm code from installing its own SIGTSTP
>    handler; (temporarily) setting SIG_IGN at Gpm_Open() time would achieve
>    this.
> 
>    Again this is a workaround specific to gpm, some other code out of
>    ncurses' control can create the same kind of problem.
>    Also, the gpm SIGTSTP handler, besides creating a problem, does something
>    useful; its function would have to be duplicated in ncurses code.
> 
> 3. Change ncurses code (back) to deal with interrupted read() system call as
>    required.
> 
>    Obviously this is the preferred solution.

No, as it would break window resizing.

> 4. Change all applications where this may occur to somehow detect the
>    situation, and work around it.

I think this is more likely. In fact other curses implementations do
return ERR on EINTR.

>    Applications shouldn't have to bother with this kind of low-level
>    stuff, that's one reason why they use higher-level functions instead
>    of doing read() etc. directly.  [Although they may still have to
>    anticipate this situation in order to be portable to other curses
>    implemetations.]

But read does return error on signal for some purpose. The problem with
getch is that it is not possible to determine the error reason. Maybe
it is possible to clear errno before getch and check it for EINTR after,
but this looks like unportable hack.

> Problem 1b
> ----------
>      Directly after suspending and resuming once, without a following
>      keystroke - that is:  ^Z, fg
>      Mouse actions don't work (have no effect, except the default mouse
>      cursor movement)
> 
> This is also caused by the non-restarting SIGTSTP handler installed by
> gpm.  What gets interrupted by the first ^Z is actually the _nc_timed_wait()
> in fifo_push(), it returns prematurely instead of waiting until new
> input is actually available.  This also happens to 'protect' the read()

In fact input subsystem of ncurses is rusty, and needs to be redesigned.
I think that timestamped queues of events would be appropriate.

> further below in fifo_push(), it is the reason why Problem 1a only
> becomes visible on the *second* interrupt.

This _is_ a bug because of inconsistency. getch should exit with ERR if
timed_wait fails. (or should ignore EINTR everywhere, which is arguably)

> Possible solutions: 1., 2. as above, or 
> 
> 3. Change ncurses code to deal with interrupted system call in
>    _nc_timed_wait() as required.
> 
>    Obviously this is the preferred solution.

Not exactly, for signal handling purpose.

   Alexander.