[PATCH v2] Cygwin: sigtimedwait: Fix segfault when timeout is used

[Corinna Vinschen]

On Nov 22 14:37, Christian Franke wrote:
> Corinna Vinschen wrote:
> > On Nov 20 22:00, Takashi Yano wrote:
> > > On Tue, 19 Nov 2024 11:51:17 +0100
> > > Corinna Vinschen wrote:
> > > > Maybe we can utilize WaitOnAddress, kind of like this?
> > > > 
> > > > sigwait_common, just the fallthrough snippet:
> > > > 
> > > >    +       /* sigpacket::process() already started.
> > > >    +          Go through to WAIT_SIGNALED case. */
> > > >    +       _my_tls.unlock ();
> > > >    +       sigset_t compare = 0;
> > > >    +       WaitOnAddress (&_my_tls.sigwait_mask, &compare,
> > > >    +                      sizeof (sigset_t), INFINITE);
> > > >    +       _my_tls.sigwait_mask = 0;
> > > >    +       fallthrough;
> > > > 
> > > > sigpacket::process():
> > > > 
> > > > @@ -1457,6 +1457,7 @@ sigpacket::process ()
> > > >     bool issig_wait = false;
> > > >     struct sigaction& thissig = global_sigs[si.si_signo];
> > > >     void *handler = have_execed ? NULL : (void *) thissig.sa_handler;
> > > > +  sigset_t orig_wait_mask = 0;
> > > >     threadlist_t *tl_entry = NULL;
> > > >     _cygtls *tls = NULL;
> > > > @@ -1527,11 +1528,15 @@ sigpacket::process ()
> > > >     if ((HANDLE) *tls)
> > > >       tls->signal_debugger (si);
> > > > -  if (issig_wait)
> > > > +  tls->lock ();
> > > > +  if (issig_wait && tls->sigwait_mask != 0)
> > > >       {
> > > > +      orig_wait_mask = tls->sigwait_mask;
> > > >         tls->sigwait_mask = 0;
> > > > +      tls->unlock ();
> > > >         goto dosig;
> > > >       }
> > > > +  tls->unlock ();
> > > >     if (handler == SIG_IGN)
> > > >       {
> > > > @@ -1606,6 +1611,11 @@ dosig:
> > > >     /* Dispatch to the appropriate function. */
> > > >     sigproc_printf ("signal %d, signal handler %p", si.si_signo, handler);
> > > >     rc = setup_handler (handler, thissig, tls);
> > > > +  if (orig_wait_mask)
> > > > +    {
> > > > +      tls->sigwait_mask = orig_wait_mask;
> > > > +      WakeByAddressAll (&tls->sigwait_mask);
> > > > +    }
> > > >   done:
> > > >     cygheap->unlock_tls (tl_entry);
> > > > 
> > > > Mind, that's just an idea.  There may be a simpler way to do this.
> > > > 
> > > > Alternatively we can just fallback to your version 1.
> > > Using WaitOnAddress() may be nice idea, however, I prefer my v1 patch.
> > > It's simpler and the intent of the code is clearer, isn't it?
> > And somehow an iteration of the above code doesn't actually fix the
> > problem, your original patch does.  So please push.
> 
> Stress-ng upstream recently re-enabled usage of
> pthread_sigqueue+sigtimedwait on Cygwin. If such a build is used with
> cygwin1.dll 26144e40, 'stress-ng --pthread' does no longer report any
> SIGSEGV errors.

Thanks for your feedback!

Corinna