Re: [PATCH] Readline: Cleanup some warnings

[Eli Zaretskii <eliz@gnu.org>]

> Cc: tom@tromey.com, gdb-patches@sourceware.org
> From: Pedro Alves <palves@redhat.com>
> Date: Wed, 20 Mar 2019 17:50:36 +0000
> 
> On 03/20/2019 05:38 PM, Eli Zaretskii wrote:
> >> Cc: tom@tromey.com, gdb-patches@sourceware.org
> >> From: Pedro Alves <palves@redhat.com>
> >> Date: Wed, 20 Mar 2019 15:46:47 +0000
> >>
> >>> I agree: the right solution would be for the Readline's SIGINT handler
> >>> to stop the main thread (e.g., by using SuspendThread).
> >>
> >> I don't sure how having the SIGINT handler stop the main thread is
> >> a 100% correct solution.  By the time you stop it, the main thread can well
> >> be already running readline code, halfway through updating some data
> >> structure, even if the mainline code disabled SIGINT temporarily,
> >> with _rl_block_sigint, because by the time the mainline code calls 
> >> _rl_block_sigint, the SIGINT thread may have already have been spawned.
> > 
> > How is this different from what happens on Posix platforms, where the
> > SIGINT handler can be invoked at any moment, while Readline might be
> > doing anything at all?
> 
> On Posix platforms the signal handler runs in the same thread as the
> mainline code.  There's _never_ any parallel execution.  When the
> mainline code calls _rl_block_sigint, you're absolutely sure that the
> mainline code that follows will not be running in parallel with
> a SIGINT handler.
> 
> > 
> >> Also, you're not ever supposed to use SuspendThread for synchronization, I
> >> believe.
> > 
> > We are also not supposed to mix CRT and Win32 API functions, but we do
> > that all the time.
> 
> That's not at all the same thing, I'm afraid.  We're not talking about
> "in theory you shouldn't, but in practice it's OK.".  We're talking about
> the very real fact that you cannot use it to synchronize correctly,
> as shown in the blog post I pasted below.  If you're using the function
> to pause a thread, and then do things, but the thread doesn't actually
> pause before you do things, then you have a problem.
> 
> > 
> >> MSDN at <https://docs.microsoft.com/en-us/windows/desktop/api/processthreadsapi/nf-processthreadsapi-suspendthread> says:
> >>
> >>  "This function is primarily designed for use by debuggers. It is not intended
> >>  to be used for thread synchronization."
> >>
> >> And here <https://devblogs.microsoft.com/oldnewthing/?p=44743> it says:
> >>
> >>   "The Suspend­Thread function tells the scheduler to suspend the thread
> >>   but does not wait for an acknowledgment from the scheduler that the suspension
> >>   has actually occurred."
> > 
> > GNU Make does it for many years, and I have yet to hear a single
> > complaint about that part.
> 
> Not hearing a complaint does not mean that the problem isn't real.
> It probably simply means that the race window is narrow, so the number
> of people that observe any issue, _and_ are willing to report an issue
> is small, and also, is someone notices something odd, it may be simply
> impossible to tell what happened and realize what the problem was,
> after the fact.

I'm sorry I brought this up.  I should have known better.