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.
Am I overlooking something?
As for volatile, personally, I have never had any problems by
not marking variables that are accessed by multiple threads as
volatile. Do you have any example that causes problem due to
lack of volatile other than, say, hardware registers?
--
Takashi Yano <takashi.y...@nifty.ne.jp>
