Re: [PATCH] Readline: Cleanup some warnings

[Eli Zaretskii <eliz at gnu dot org>]

> Cc: gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 19 Mar 2019 19:02:43 +0000
> 
> > https://sourceware.org/ml/gdb-patches/2008-02/msg00423.html

Caveat: I didn't yet read that thread myself.

> Hmmm, 
> 
> Daniel wrote:
> 
> > GDB has several SIGINT handlers which call longjmp.  This is
> > problematic for at least two reasons.  One is that we could be in the
> > middle of something unwise to longjmp out of, for instance malloc.  In
> > practice, this never happens because we're usually waiting for I/O
> > when one of the relevant handlers is invoked, but there are a number
> > of places where it could definitely happen.
> 
> That was indeed true back then, but since then, immediate_quit
> was completely eliminated, and we no longer longjmp from signal
> handlers anymore, since:
>  https://sourceware.org/ml/gdb-patches/2016-03/msg00351.html
> 
> Daniel wrote:
> 
> > My goals in fixing this were to hide the Windows ugliness, and to fit
> > in nicely with GDB's asynchronous event loop.  Since we do not return
> > to the primary event loop during target actions (for the current,
> > non-async GDB), I couldn't rely on the event loop entirely.  But I
> > could use the same token mechanism and thus share the bodies of
> > handlers for async mode with the Windows case.
> >
> > The new interface is gdb_call_async_signal_handler.  SIGINT handlers,
> 
> This interface he mentioned, gdb_call_async_signal_handler, was
> eliminated by that series too:
> 
>  https://sourceware.org/ml/gdb-patches/2016-03/msg00347.html
> 
> So all that's left is that little readline hack, it seems:
> 
>   /* With multi-threaded SIGINT handling, there is a race between the
>      readline signal handler and GDB.  It may still be in
>      rl_prep_terminal in another thread.  Do not return until it is
>      done; we can check the state here because we never longjmp from
>      signal handlers on Windows.  */
>   while (RL_ISSTATE (RL_STATE_SIGHANDLER))
>     Sleep (1);
> 
> (Curiously, that bit only appeared in a later version of Dan's patch,
> here: https://sourceware.org/ml/gdb-patches/2008-03/msg00034.html)
> 
> I'm not seeing why we'd still need that bit, but then again,
> I'm not seeing why it was needed in the first place.
> The signal handler could run concurrently with gdb at any other
> point in the gdb code, not just here, so at any point we
> call into readline, we can be running readline code in parallel
> with a signal handler touching readline's state.  It sounds like
> that should be a readline problem to worry about.
> 
> That could be related to the fact that readline's signal handler
> overrides gdb's, does its thing, and then calls gdb's signal
> handler manually?  If the WaitForSingleObject call had already
> woken up, then gdb's signal handler has already run and SetEvent
> on sigint_event.  Then the code would go and run the deferred
> signal handler.  In the remote case, that handler would
> issue prompt "Give up (and stop debugging it)? (y or n)" prompt,
> and if that is running in parallel with readline's signal
> handler still calling rl_prep_terminal, bad things would happen.

Not sure if the above refers to what I wanted to say, but: as I'm sure
you know, SIGINT handlers on Windows run in a separate thread, created
by the OS, so a Readline SIGINT handler could ruin in parallel both
with Readline's other code and in parallel with GDB's code, depending
on when exactly did the user type Ctrl-C.  In a few cases where it was
important to emulate Posix behavior in order not to step on the troes
of the mainline code, I needed to stop the main thread while the
SIGINT handler was running.  Could it be that the code we are
discussing does something similar?

> But again, why isn't that a readline problem, instead of
> a gdb problem?

I agree: the right solution would be for the Readline's SIGINT handler
to stop the main thread (e.g., by using SuspendThread).

> I'm still puzzled on why this isn't a readline issue.  Shouldn't
> readline's Windows signal handler be synchronizing with mainline
> code such that if a signal handler is running, mainline calls into
> readline would block?

Yes, I think so.

> I think there must be something else to this.

Maybe.  I will try to read that discussion soon.

Thanks.