Menu
▾
▴
#105 Use signalfd() on linux
This patch will use the linux signalfd() interface internally for signal events instead of the traditional sigaction() method. Shorter, simpler code.
Monitoring different signals in different event bases should be okay. Waiting for the same signal in different bases isn't (The manual says not to do this, but there's a test case that does anyways...). Mixing traditional signal()/sigaction() handling and this should be okay as long as signal sets don't overlap; when they do, the event handler will have priority, since signals monitored by signalfd get masked so they don't trigger regular handlers.
generated by git format-patch
Neat! I've wanted to do this for a while.
I'd be curious to see you compare and contrast this approach with the (probably not-working) implementation I started in https://github.com/nmathewson/Libevent/commit/a01347e9ac31b8e9b234d01eaf1c7e4edf8519e9 . Is there anything fundamental that we do differently? Do you think there is anything that we should take from my patch there?
I think the biggest difference from a quick look at your version is that I'm saving the existing signal mask in evsig_init() and restoring it in evsig_dealloc() (Which also means, come to think of it, that multiple bases and traditional signal handler functions aren't going to play nice. Mixing different signal handling techniques probably shouldn't be done anyways for the sake of the programmer's sanity.).
I try to avoid threads in favor of event systems or separate processes, so I wasn't aware that sigprocmask() and pthreads don't get along. There is a pthread_sigprocmask() that looks like it could be used instead... I should also double-check with threads in mind and make sure I don't have to add more locked regions. An event_base isn't supposed to be used by multiple threads at the same time, right?
So, the issue with pthread_sigprocmask (as near as I can tell) is figuring out whether we need it in time to call it. Last time I looked, the rules were pretty confusing, and would maybe require us to provide a function that got called for every new thread. If so, that's a problem; it would break existing libevent programs. Let's hope we do't have to do that.
One thing to consider :The rule about interactions between pthreads and sigprocmask is a pthreads rule, but I don't know whether it matters on Linux or not. If not, then we can just use sigprocmask, since the presence of signalfd already means we're Linux (I believe, right?)
It is totally permissible or an event_base to be used by multiple threads at once. That is to say, at most one thread at a time is currently allowed to be running the event loop of an event_base ... but it *is* allowed for other threads to affect the event_base by calling event_add, event_del, and event_active on events associated with that base; by using event_base_once, event_base_loopexit, and event_base_loopbreak; and by similar functions.
Also, If this is going to provide different behavior for some programs that accidentally worked before, I wonder if there shouldn't be a run-time way to disable it via the event_base_config stuff.
Hello, Shawn! Any progress here?
Unfortunately, I haven't had time to sit down and work on thread safety yet. Between work and the holidays...
So, a quick look at pthread_sigmask from glibc does inspire confidence:
http://glibc.sourcearchive.com/documentation/2.9/pthread__sigmask_8c-source.html
It looks to me like all it is doing is making sure that we never clear a couple of signals (SIGCANCEL, SIGSETXID) that are used for threading, Then it calls either rt_sigprocmask or sigprocmask. The rt_sigprocmask call doesn't seem to have different semantics to me, except that it can handle a variable number of signals. (It also grabs some locks differently.)
So on Linux, I *think* we might be clear here.
I finally got some time to sit down and look at this more, and I think the whole idea might be a dead end. pthread_sigmask() changes the signal mask of the calling thread, not the process as a whole. So adding a watched signal in a thread that's not the one doing the polling on that event_base will have problems, since the mask in the polling thread won't be right. I suspect that changing things so only the polling thread ever uses pthread_sigmask() will be enough of a pain that it looses any benefits to using signalfd() at all. The other thread would have to queue an event in the polling thread's queue, which would run later, introducing new timing issues...
All this is only a problem in multi threaded code where a separate thread from the polling one tries to add or delete signal events. What about using signalfd() for linux libevent instances configured without threading support, and the current signal handler method otherwise? I'm not sure how often that particular combination arises in practice, though.
Hm. Actually, reading the Linux documentation, I think sigprocmask only affects the calling thread too.
But here's a crazy idea I just had! If you call signal() with a dummy handler, then signalfd still works fine, but other threads won't actually get interrupted by the signal. That is, if you do "void ignore(int signum) {} ... signal(SIGINT, ignore)" in place of using pthread_sigprocmask, it is a process-wide effect that still allows signalfd to work.
This isn't necessarily portable, but of course neither is signalfd.
Otherwise, If we're going to put this feature in, I think the only right way to do it is to make it an off-by-default thing that never turns on unless the user explicitly says "I will make sure to call sigprocmask or pthread_sigprocmask as appropriate in any threads not running the event loop."