[PATCH v2] Cygwin: signal: Do not handle signal when __SIGFLUSHFAST is sent

[Corinna Vinschen]

On Jan 19 19:42, Takashi Yano wrote:
> Hi Corinna,
> 
> On Sun, 19 Jan 2025 11:49:58 +0900
> Takashi Yano wrote:
> > On Sat, 18 Jan 2025 17:06:50 -0800 (PST)
> > Jeremy Drake wrote:
> > > On Sat, 18 Jan 2025, Takashi Yano wrote:
> > > 
> > > > While debugging this problem, I encountered another hang issue,
> > > > which is fixed by:
> > > > 0001-Cygwin-signal-Avoid-frequent-tls-lock-unlock-for-SIG.patch
> > > 
> > > I'm concerned about this patch.  There's a window where current_sig could
> > > be changed after exiting the while, before the lock is acquired by
> > > cygheap->find_tls (_main_tls);  Should current_sig be rechecked after the
> > > lock is acquired to make sure that hasn't happened?  Also, does
> > > current_sig need to be volatile, or is yield a sufficient fence for the
> > > compiler to know other threads may have changed the value?
> > 
> > Thanks for pointing out this. You are right if othre threads may
> > set current_sig to non-zero value. Current cygwin sets current_sig
> > to non-zero only in 
> > _cygtls::interrupt_setup()
> > and
> > _cygtls::handle_SIGCONT()
> > both are called from sigpacket::process() as follows.
> > 
> > wait_sig()->
> >  sigpacket::process() +-> sigpacket::setup_handler() -> _cygtls::interrupt_setup()
> >                       \-> _cygtls::handle_SIGCONT()
> > 
> > wait_sig() is a thread which handle received signals, so other
> > threads than wait_sig() thread do not set the current_sig to non-zero.
> > That is, other threads set current_sig only to zero. Therefore,
> > I think we don't have to guard checking current_sig value by lock.
> > The only thing we shoud guard is the following case.
> > 
> > [wait_sig()]               [another thread]
> > current_sig = SIGCONT;
> >                            current_sig = 0;
> > set_signal_arrived();
> > 
> > So, we should place current_sig = SIGCONT and set_signal_arrived()
> > inside the lock.
> 
> I think the lock necessary here is _cygtls::lock(), isn't it?
> Because the _cygtls::call_signal_handler() uses _cygtls::locl().
> I'm asking you because you introduced the find_tls() lock first
> in the commit:

Yeah, _cygtls::lock() of the target thread should be right.
The mutex in find_tls was for guarding threadlist_t, not the
thread's _cygtls.


Corinna