From: Niels Provos Date: Fri, 11 Jul 2008 15:49:04 +0000 (+0000) Subject: support multiple events listening on the same signal; make signals regular events... X-Git-Tag: release-2.0.1-alpha~239 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f7e61870e95efded87d17f2cf29ec9af815b9d30;p=libevent support multiple events listening on the same signal; make signals regular events that go on the same event queue svn:r901 --- diff --git a/ChangeLog b/ChangeLog index f3e66962..f80316cb 100644 --- a/ChangeLog +++ b/ChangeLog @@ -117,6 +117,7 @@ Changes in current version: o Detect CLOCK_MONOTONIC at runtime for evdns; anonymous bug report o Various HTTP correctness fixes from Scott Lamb o Fix a bug where deleting signals with the kqueue backend would cause subsequent adds to fail + o Support multiple events listening on the same signal; make signals regular events that go on the same event queue. Changes in 1.4.0: o allow \r or \n individually to separate HTTP headers instead of the standard "\r\n"; from Charles Kerr. diff --git a/event-internal.h b/event-internal.h index a8a2a2c4..84e898ac 100644 --- a/event-internal.h +++ b/event-internal.h @@ -39,7 +39,6 @@ extern "C" { /* map union members back */ /* mutually exclusive */ -#define ev_next _ev.ev_io.ev_next #define ev_signal_next _ev.ev_signal.ev_signal_next /* used only by signals */ diff --git a/event.c b/event.c index 6fcfcd3d..bc307344 100644 --- a/event.c +++ b/event.c @@ -244,7 +244,6 @@ event_base_new_with_config(struct event_config *cfg) min_heap_ctor(&base->timeheap); TAILQ_INIT(&base->eventqueue); - TAILQ_INIT(&base->sig.signalqueue); base->sig.ev_signal_pair[0] = -1; base->sig.ev_signal_pair[1] = -1; @@ -678,7 +677,7 @@ event_base_loop(struct event_base *base, int flags) struct timeval *tv_p; int res, done; - if(!TAILQ_EMPTY(&base->sig.signalqueue)) + if (&base->sig.ev_signal_added) evsignal_base = base; done = 0; while (!done) { @@ -939,13 +938,11 @@ event_pending(struct event *ev, short event, struct timeval *tv) int flags = 0; if (ev->ev_flags & EVLIST_INSERTED) - flags |= (ev->ev_events & (EV_READ|EV_WRITE)); + flags |= (ev->ev_events & (EV_READ|EV_WRITE|EV_SIGNAL)); if (ev->ev_flags & EVLIST_ACTIVE) flags |= ev->ev_res; if (ev->ev_flags & EVLIST_TIMEOUT) flags |= EV_TIMEOUT; - if (ev->ev_flags & EVLIST_SIGNAL) - flags |= EV_SIGNAL; event &= (EV_TIMEOUT|EV_READ|EV_WRITE|EV_SIGNAL); @@ -1026,7 +1023,7 @@ event_add_internal(struct event *ev, const struct timeval *tv) * removes it from the active list. */ if ((ev->ev_flags & EVLIST_ACTIVE) && (ev->ev_res & EV_TIMEOUT)) { - if (ev->ev_flags & EVLIST_SIGNAL) { + if (ev->ev_events & EV_SIGNAL) { /* See if we are just active executing * this event in a loop */ @@ -1049,16 +1046,11 @@ event_add_internal(struct event *ev, const struct timeval *tv) event_queue_insert(base, ev, EVLIST_TIMEOUT); } - if ((ev->ev_events & (EV_READ|EV_WRITE)) && + if ((ev->ev_events & (EV_READ|EV_WRITE|EV_SIGNAL)) && !(ev->ev_flags & (EVLIST_INSERTED|EVLIST_ACTIVE))) { res = evsel->add(evbase, ev); if (res != -1) event_queue_insert(base, ev, EVLIST_INSERTED); - } else if ((ev->ev_events & EV_SIGNAL) && - !(ev->ev_flags & EVLIST_SIGNAL)) { - res = evsel->add(evbase, ev); - if (res != -1) - event_queue_insert(base, ev, EVLIST_SIGNAL); } /* if we are not in the right thread, we need to wake up the loop */ @@ -1104,7 +1096,7 @@ event_del_internal(struct event *ev) assert(!(ev->ev_flags & ~EVLIST_ALL)); /* See if we are just active executing this event in a loop */ - if (ev->ev_flags & EVLIST_SIGNAL) { + if (ev->ev_events & EV_SIGNAL) { if (ev->ev_ncalls && ev->ev_pncalls) { /* Abort loop */ *ev->ev_pncalls = 0; @@ -1120,9 +1112,6 @@ event_del_internal(struct event *ev) if (ev->ev_flags & EVLIST_INSERTED) { event_queue_remove(base, ev, EVLIST_INSERTED); res = evsel->del(evbase, ev); - } else if (ev->ev_flags & EVLIST_SIGNAL) { - event_queue_remove(base, ev, EVLIST_SIGNAL); - res = evsel->del(evbase, ev); } /* if we are not in the right thread, we need to wake up the loop */ @@ -1158,7 +1147,7 @@ event_active_internal(struct event *ev, int res, short ncalls) ev->ev_res = res; - if (ev->ev_flags & EVLIST_SIGNAL) { + if (ev->ev_events & EV_SIGNAL) { ev->ev_ncalls = ncalls; ev->ev_pncalls = NULL; } @@ -1298,9 +1287,6 @@ event_queue_remove(struct event_base *base, struct event *ev, int queue) case EVLIST_TIMEOUT: min_heap_erase(&base->timeheap, ev); break; - case EVLIST_SIGNAL: - TAILQ_REMOVE(&base->sig.signalqueue, ev, ev_signal_next); - break; default: event_errx(1, "%s: unknown queue %x", __func__, queue); } @@ -1335,9 +1321,6 @@ event_queue_insert(struct event_base *base, struct event *ev, int queue) min_heap_push(&base->timeheap, ev); break; } - case EVLIST_SIGNAL: - TAILQ_INSERT_TAIL(&base->sig.signalqueue, ev, ev_signal_next); - break; default: event_errx(1, "%s: unknown queue %x", __func__, queue); } diff --git a/evsignal.h b/evsignal.h index 65297db4..b6cedc7f 100644 --- a/evsignal.h +++ b/evsignal.h @@ -34,11 +34,11 @@ typedef void (*ev_sighandler_t)(int); struct evsignal_info { - struct event_list signalqueue; struct event ev_signal; - evutil_socket_t ev_signal_pair[2]; + evutil_socket_t ev_signal_pair[2]; int ev_signal_added; volatile sig_atomic_t evsignal_caught; + struct event_list evsigevents[NSIG]; sig_atomic_t evsigcaught[NSIG]; #ifdef HAVE_SIGACTION struct sigaction **sh_old; diff --git a/http.c b/http.c index b283685d..d570ce7f 100644 --- a/http.c +++ b/http.c @@ -1575,7 +1575,8 @@ evhttp_read_firstline(struct evhttp_connection *evcon, res = evhttp_parse_firstline(req, bufferevent_get_input(evcon->bufev)); if (res == DATA_CORRUPTED) { /* Error while reading, terminate */ - event_debug(("%s: bad header lines on %d\n", __func__, fd)); + event_debug(("%s: bad header lines on %d\n", + __func__, evcon->fd)); evhttp_connection_fail(evcon, EVCON_HTTP_INVALID_HEADER); return; } else if (res == MORE_DATA_EXPECTED) { diff --git a/include/event2/event_struct.h b/include/event2/event_struct.h index 68ef1596..3c0d9b38 100644 --- a/include/event2/event_struct.h +++ b/include/event2/event_struct.h @@ -78,17 +78,13 @@ struct { \ struct event_base; struct event { TAILQ_ENTRY (event) (ev_active_next); + TAILQ_ENTRY (event) (ev_next); int min_heap_idx; /* for managing timeouts */ struct event_base *ev_base; evutil_socket_t ev_fd; union { - /* used by io events */ - struct { - TAILQ_ENTRY (event) (ev_next); - } ev_io; - /* used by signal events */ struct { TAILQ_ENTRY (event) (ev_signal_next); diff --git a/kqueue.c b/kqueue.c index 6627171e..d61cb365 100644 --- a/kqueue.c +++ b/kqueue.c @@ -44,6 +44,7 @@ #include #include #include +#include #ifdef HAVE_INTTYPES_H #include #endif @@ -71,6 +72,7 @@ struct kqop { struct kevent *changes; int nchanges; struct kevent *events; + struct event_list evsigevents[NSIG]; int nevents; int kq; pid_t pid; @@ -97,7 +99,7 @@ const struct eventop kqops = { static void * kq_init(struct event_base *base) { - int kq; + int i, kq; struct kqop *kqueueop; if (!(kqueueop = mm_calloc(1, sizeof(struct kqop)))) @@ -129,6 +131,11 @@ kq_init(struct event_base *base) } kqueueop->nevents = NEVENT; + /* we need to keep track of multiple events per signal */ + for (i = 0; i < NSIG; ++i) { + TAILQ_INIT(&kqueueop->evsigevents[i]); + } + /* Check for Mac OS X kqueue bug. */ kqueueop->changes[0].ident = -1; kqueueop->changes[0].filter = EVFILT_READ; @@ -257,8 +264,6 @@ kq_dispatch(struct event_base *base, void *arg, struct timeval *tv) return (-1); } - ev = (struct event *)events[i].udata; - if (events[i].filter == EVFILT_READ) { which |= EV_READ; } else if (events[i].filter == EVFILT_WRITE) { @@ -270,11 +275,20 @@ kq_dispatch(struct event_base *base, void *arg, struct timeval *tv) if (!which) continue; - if (!(ev->ev_events & EV_PERSIST)) - ev->ev_flags &= ~EVLIST_X_KQINKERNEL; + if (events[i].filter == EVFILT_SIGNAL) { + struct event_list *head = + (struct event_list *)events[i].udata; + TAILQ_FOREACH(ev, head, ev_signal_next) { + event_active(ev, which, events[i].data); + } + } else { + ev = (struct event *)events[i].udata; + + if (!(ev->ev_events & EV_PERSIST)) + ev->ev_flags &= ~EVLIST_X_KQINKERNEL; - event_active(ev, which | (ev->ev_events & EV_ET), - ev->ev_events & EV_SIGNAL ? events[i].data : 1); + event_active(ev, which | (ev->ev_events & EV_ET), 1); + } } return (0); @@ -289,25 +303,30 @@ kq_add(void *arg, struct event *ev) if (ev->ev_events & EV_SIGNAL) { int nsignal = EVENT_SIGNAL(ev); - struct timespec timeout = { 0, 0 }; - memset(&kev, 0, sizeof(kev)); - kev.ident = nsignal; - kev.filter = EVFILT_SIGNAL; - kev.flags = EV_ADD; - if (!(ev->ev_events & EV_PERSIST)) - kev.flags |= EV_ONESHOT; - kev.udata = PTR_TO_UDATA(ev); - - /* Be ready for the signal if it is sent any time between - * now and the next call to kq_dispatch. */ - if (kevent(kqop->kq, &kev, 1, NULL, 0, &timeout) == -1) - return (-1); - - if (_evsignal_set_handler(ev->ev_base, nsignal, - kq_sighandler) == -1) - return (-1); + assert(nsignal >= 0 && nsignal < NSIG); + if (TAILQ_EMPTY(&kqop->evsigevents[nsignal])) { + struct timespec timeout = { 0, 0 }; + + memset(&kev, 0, sizeof(kev)); + kev.ident = nsignal; + kev.filter = EVFILT_SIGNAL; + kev.flags = EV_ADD; + kev.udata = PTR_TO_UDATA(&kqop->evsigevents[nsignal]); + + /* Be ready for the signal if it is sent any + * time between now and the next call to + * kq_dispatch. */ + if (kevent(kqop->kq, &kev, 1, NULL, 0, &timeout) == -1) + return (-1); + + if (_evsignal_set_handler(ev->ev_base, nsignal, + kq_sighandler) == -1) + return (-1); + } + TAILQ_INSERT_TAIL(&kqop->evsigevents[nsignal], ev, + ev_signal_next); ev->ev_flags |= EVLIST_X_KQINKERNEL; return (0); } @@ -366,18 +385,24 @@ kq_del(void *arg, struct event *ev) int nsignal = EVENT_SIGNAL(ev); struct timespec timeout = { 0, 0 }; - memset(&kev, 0, sizeof(kev)); - kev.ident = nsignal; - kev.filter = EVFILT_SIGNAL; - kev.flags = EV_DELETE; + assert(nsignal >= 0 && nsignal < NSIG); + TAILQ_REMOVE(&kqop->evsigevents[nsignal], ev, ev_signal_next); + if (TAILQ_EMPTY(&kqop->evsigevents[nsignal])) { + memset(&kev, 0, sizeof(kev)); + kev.ident = nsignal; + kev.filter = EVFILT_SIGNAL; + kev.flags = EV_DELETE; - /* Because we insert signal events immediately, we need to - * delete them immediately, too */ - if (kevent(kqop->kq, &kev, 1, NULL, 0, &timeout) == -1) - return (-1); - - if (_evsignal_restore_handler(ev->ev_base, nsignal) == -1) - return (-1); + /* Because we insert signal events + * immediately, we need to delete them + * immediately, too */ + if (kevent(kqop->kq, &kev, 1, NULL, 0, &timeout) == -1) + return (-1); + + if (_evsignal_restore_handler(ev->ev_base, + nsignal) == -1) + return (-1); + } ev->ev_flags &= ~EVLIST_X_KQINKERNEL; return (0); diff --git a/signal.c b/signal.c index 87fcdf3b..2746bc09 100644 --- a/signal.c +++ b/signal.c @@ -97,12 +97,15 @@ evsignal_cb(evutil_socket_t fd, short what, void *arg) void evsignal_init(struct event_base *base) { + int i; + /* * Our signal handler is going to write to one end of the socket * pair to wake up our event loop. The event loop then scans for * signals that got delivered. */ - if (evutil_socketpair(AF_UNIX, SOCK_STREAM, 0, base->sig.ev_signal_pair) == -1) + if (evutil_socketpair( + AF_UNIX, SOCK_STREAM, 0, base->sig.ev_signal_pair) == -1) event_err(1, "%s: socketpair", __func__); FD_CLOSEONEXEC(base->sig.ev_signal_pair[0]); @@ -111,6 +114,9 @@ evsignal_init(struct event_base *base) base->sig.sh_old_max = 0; base->sig.evsignal_caught = 0; memset(&base->sig.evsigcaught, 0, sizeof(sig_atomic_t)*NSIG); + /* initialize the queues for all events */ + for (i = 0; i < NSIG; ++i) + TAILQ_INIT(&base->sig.evsigevents[i]); evutil_make_socket_nonblocking(base->sig.ev_signal_pair[0]); @@ -189,20 +195,26 @@ evsignal_add(struct event *ev) struct evsignal_info *sig = &ev->ev_base->sig; evsignal = EVENT_SIGNAL(ev); + assert(evsignal >= 0 & evsignal < NSIG); + if (TAILQ_EMPTY(&sig->evsigevents[evsignal])) { + event_debug(("%s: %p: changing signal handler", __func__, ev)); + if (_evsignal_set_handler( + base, evsignal, evsignal_handler) == -1) + return (-1); - event_debug(("%s: %p: changing signal handler", __func__, ev)); - if (_evsignal_set_handler(base, evsignal, evsignal_handler) == -1) - return (-1); - - /* catch signals if they happen quickly */ - evsignal_base = base; + /* catch signals if they happen quickly */ + evsignal_base = base; - if (!sig->ev_signal_added) { - if (event_add(&sig->ev_signal, NULL)) - return (-1); - sig->ev_signal_added = 1; + if (!sig->ev_signal_added) { + if (event_add(&sig->ev_signal, NULL)) + return (-1); + sig->ev_signal_added = 1; + } } + /* multiple events may listen to the same signal */ + TAILQ_INSERT_TAIL(&sig->evsigevents[evsignal], ev, ev_signal_next); + return (0); } @@ -240,8 +252,21 @@ _evsignal_restore_handler(struct event_base *base, int evsignal) int evsignal_del(struct event *ev) { + struct event_base *base = ev->ev_base; + struct evsignal_info *sig = &base->sig; + int evsignal = EVENT_SIGNAL(ev); + + assert(evsignal >= 0 & evsignal < NSIG); + + /* multiple events may listen to the same signal */ + TAILQ_REMOVE(&sig->evsigevents[evsignal], ev, ev_signal_next); + + if (!TAILQ_EMPTY(&sig->evsigevents[evsignal])) + return (0); + event_debug(("%s: %p: restoring signal handler", __func__, ev)); - return _evsignal_restore_handler(ev->ev_base, EVENT_SIGNAL(ev)); + + return (_evsignal_restore_handler(ev->ev_base, EVENT_SIGNAL(ev))); } static void @@ -249,7 +274,7 @@ evsignal_handler(int sig) { int save_errno = errno; - if(evsignal_base == NULL) { + if (evsignal_base == NULL) { event_warn( "%s: received signal %d, but have no base configured", __func__, sig); @@ -271,29 +296,39 @@ evsignal_handler(int sig) void evsignal_process(struct event_base *base) { - struct event *ev; + struct evsignal_info *sig = &base->sig; + struct event *ev, *next_ev; sig_atomic_t ncalls; - + int i; + base->sig.evsignal_caught = 0; - TAILQ_FOREACH(ev, &base->sig.signalqueue, ev_signal_next) { - ncalls = base->sig.evsigcaught[EVENT_SIGNAL(ev)]; - if (ncalls) { + for (i = 1; i < NSIG; ++i) { + ncalls = sig->evsigcaught[i]; + if (ncalls == 0) + continue; + + for (ev = TAILQ_FIRST(&sig->evsigevents[i]); + ev != NULL; ev = next_ev) { + next_ev = TAILQ_NEXT(ev, ev_signal_next); if (!(ev->ev_events & EV_PERSIST)) event_del(ev); event_active(ev, EV_SIGNAL, ncalls); - base->sig.evsigcaught[EVENT_SIGNAL(ev)] = 0; } + + sig->evsigcaught[i] = 0; } } void evsignal_dealloc(struct event_base *base) { - if(base->sig.ev_signal_added) { + int i = 0; + if (base->sig.ev_signal_added) { event_del(&base->sig.ev_signal); base->sig.ev_signal_added = 0; } - assert(TAILQ_EMPTY(&base->sig.signalqueue)); + for (i = 0; i < NSIG; ++i) + assert(TAILQ_EMPTY(&base->sig.evsigevents[0])); EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[0]); base->sig.ev_signal_pair[0] = -1; diff --git a/test/regress.c b/test/regress.c index b19402da..1fd7c30a 100644 --- a/test/regress.c +++ b/test/regress.c @@ -577,6 +577,36 @@ test_simplesignal(void) cleanup_test(); } +static void +test_multiplesignal(void) +{ + struct event ev_one, ev_two; + struct itimerval itv; + + setup_test("Multiple signal: "); + + signal_set(&ev_one, SIGALRM, signal_cb, &ev_one); + signal_add(&ev_one, NULL); + + signal_set(&ev_two, SIGALRM, signal_cb, &ev_two); + signal_add(&ev_two, NULL); + + memset(&itv, 0, sizeof(itv)); + itv.it_value.tv_sec = 1; + if (setitimer(ITIMER_REAL, &itv, NULL) == -1) + goto skip_simplesignal; + + event_dispatch(); + + skip_simplesignal: + if (signal_del(&ev_one) == -1) + test_ok = 0; + if (signal_del(&ev_two) == -1) + test_ok = 0; + + cleanup_test(); +} + static void test_immediatesignal(void) { @@ -2261,8 +2291,9 @@ main (int argc, char **argv) test_simpletimeout(); #ifndef WIN32 - test_edgetriggered(); + test_edgetriggered(); test_simplesignal(); + test_multiplesignal(); test_immediatesignal(); #endif test_loopexit();