[Libevent-users] Sigprocmask vs pthread_sigprocmask

[Scott Lamb]

On Feb 21, 2007, at 1:29 AM, William Ahern wrote:

> On Wed, Feb 21, 2007 at 03:44:58AM -0500, Nick Mathewson wrote:
> <snip>
>> libevent, "I'm going to use pthreads; use pthread_sigmask()  
>> instead of
>> sigprocmask()."  I don't know what that interface should be, but the
>> corresponding code should be pretty simple to write.
>
> Probably should be a [global] function pointer, since an autoconf  
> check
> doesn't address the linking issue.

What about just never using sigprocmask() or pthread_sigmask()? The  
patch I sent to the list a while back fixes two bugs:

* prevents Anton Povarov's reported race in which a signal delivered  
quickly after event_signal_add() isn't handled correctly
* avoids sigprocmask(), which is not pthread-safe.

There are still several caveats with threads. Ignoring the complex  
idea of sharing an event_base between threads (need still has to be  
proven...I've been way too busy with other things to run the  
benchmarks I've been wanting to), these ones remain:

2. If you forget event_base_set on an event, it's associated with the  
latest base created. This will probably work most of the time. It'd  
be much less confusing if it consistently broke.

3. Each new base created leaks the existing ev_signal_pair descriptors.

4. Signals are delivered to whatever event loop happens to see them  
first.

6. You can't destroy an event_base.

My old patch and notes are below.

This patch does not get rid of ncalls, which I'm also thinking of  
doing, as I mentioned in an earlier message to the list.

devpoll.c      |   15 +++--------
epoll.c        |   15 +++--------
evport.c       |   17 +++----------
evsignal.h     |    8 ++----
poll.c         |   15 +++--------
select.c       |   14 +++-------
signal.c       |   74 +++++++++++++++ 
+-----------------------------------------
test/regress.c |   32 ++++++++++++++++++++++++
8 files changed, 76 insertions(+), 114 deletions(-)

There's one subtlety to it. With my change, evsignal_process() runs  
with signals unblocked, so timing is critical. The original code is  
then racy:

         TAILQ_FOREACH(ev, &signalqueue, ev_signal_next) {
                 ncalls = evsigcaught[EVENT_SIGNAL(ev)];		<-- point A
                 if (ncalls) {
                         if (!(ev->ev_events & EV_PERSIST))
                                 event_del(ev);
                         event_active(ev, EV_SIGNAL, ncalls);
                 }
         }

         memset(evsigcaught, 0, sizeof(evsigcaught));		<-- point B
         evsignal_caught = 0;					<-- point C

For any signal that is not already pending (ncalls == 0 when reaching  
its point A), if it arrives between then and its ncalls being cleared  
in point B, it will be lost. If it arrives after then but before  
evsignal_caught in point C, it will be arbitrarily delayed (until  
another signal comes along to set 	evsignal_caught).

I solved this by

* moving the evsignal_caught to before checking the individual  
signals for delivery (but after evsignal_caught is checked itself, in  
a different function).
* setting "evsigcaught[x] = 0" only when we are setting that signal  
active.

-- 
Scott Lamb <http://www.slamb.org/>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: event-signal.patch
Type: application/octet-stream
Size: 12825 bytes
Desc: not available
Url : http://monkeymail.org/archives/libevent-users/attachments/20070222/f82f94b3/event-signal.obj
-------------- next part --------------