This is the mail archive of the
cygwin-patches@cygwin.com
mailing list for the Cygwin project.
Re: Signal handling tune up.
Christopher Faylor wrote:
>
>>> >Don't we have the same problem today?
>>> >
>>> >Handler is running with current mask M1, old mask M0
>>> >New signal occurs, sigthread prepares sigsave with new mask M2, old
mask M1
>>> > but is preempted before setting sigsave.sig
>>> >Handler terminates, restores M0
>>> >sigthread resumes and starts new handler. It runs with M2 and restores
>>> >M1 (instead of M0) at end.
>>>
>>> Is this any different from UNIX? Some races in signal handling are
>>> inavoidable, IMO. I guess you could make things slightly more
>>> determinstic by setting a lock. Probably UNIX has the luxury of being
>>> able to tell the process handling a signal to stop working for a second
>>> while it fiddles with masks. We don't have any reliable way of doing
>>> that.
>>
>>Here is a safe way.
>>
>>1) change sigsave to contain the incremental mask bits, set by sigthread
>> (instead of old mask and new mask)
>
> And while you are in the process of saving the incremental bits, they
> are in the process of being changed == race. I don't see why this adds
> anything. I'm also not sure that this is posix-sanctioned behavior.
Here are the details.
Currently the sigthread executes
sigsave.oldmask = myself->getsigmask ();
sigsave.newmask = sigsave.oldmask | siga.sa_mask | SIGTOMASK (sig);
As you noted last night there is a race condition with the handler,
which may terminate and change the sigmask, so that the new handler
will run with and restore incorrect masks.
What I was suggesting is to set (instead of .oldmask and .newmask)
sigsave.deltamask = siga.sa_mask | SIGTOMASK (sig);
and to let the handler compute when it starts
newmask = myself->getsigmask () | sigsave.deltamask;
and then call set_process_mask (newmask)
Alternatively it's not even necessary to define sigsave.deltamask,
the handler can call directly
set_process_mask (myself->getsigmask () | siga.sa_mask | SIGTOMASK (sig));
although that doesn't fit that well with the current asm implementation.
The overall behavior is as per posix.
Pierre