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
