From 5ff1fd7a9934455ff88148cac0c5c97de9e912c9 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 25 Nov 2007 17:15:39 +0000 Subject: [PATCH] r14945@tombo: nickm | 2007-11-25 12:13:05 -0500 Make kqueue pass more unit tests. (Backport) svn:r545 --- ChangeLog | 3 +++ event-internal.h | 4 ++++ kqueue.c | 15 +++++++++----- signal.c | 54 ++++++++++++++++++++++++++++++++---------------- test/regress.c | 14 ++++++++++--- 5 files changed, 64 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index 871cbe43..f27d7177 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,9 @@ Changes in current version: o provide event_base_new() that does not set the current_base global o bufferevent_write now uses a const source argument; report from Charles Kerr o better documentation for event_base_loopexit; from Scott Lamb. + o Make kqueue have the same behavior as other backends when a signal is caught between event_add() and event_loop(). Previously, it would catch and ignore such signals. + o Make kqueue restore signal handlers correctly when event_del() is called. + Changes in 1.4.0-beta: 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 de9c7879..275959eb 100644 --- a/event-internal.h +++ b/event-internal.h @@ -73,6 +73,10 @@ struct event_base { } while (0) #endif /* TAILQ_FOREACH */ +int _evsignal_set_handler(struct event_base *base, int evsignal, + void (*fn)(int)); +int _evsignal_restore_handler(struct event_base *base, int evsignal); + #ifdef __cplusplus } #endif diff --git a/kqueue.c b/kqueue.c index c6dbf222..186a3dd5 100644 --- a/kqueue.c +++ b/kqueue.c @@ -59,6 +59,7 @@ #include "event.h" #include "log.h" +#include "event-internal.h" #define EVLIST_X_KQINKERNEL 0x1000 @@ -293,6 +294,7 @@ 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; @@ -301,11 +303,14 @@ kq_add(void *arg, struct event *ev) if (!(ev->ev_events & EV_PERSIST)) kev.flags |= EV_ONESHOT; kev.udata = PTR_TO_UDATA(ev); - - if (kq_insert(kqop, &kev) == -1) - return (-1); - if (signal(nsignal, kq_sighandler) == SIG_ERR) + /* 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); ev->ev_flags |= EVLIST_X_KQINKERNEL; @@ -369,7 +374,7 @@ kq_del(void *arg, struct event *ev) if (kq_insert(kqop, &kev) == -1) return (-1); - if (signal(nsignal, SIG_DFL) == SIG_ERR) + if (_evsignal_restore_handler(ev->ev_base, nsignal) == -1) return (-1); ev->ev_flags &= ~EVLIST_X_KQINKERNEL; diff --git a/signal.c b/signal.c index 4b3a488e..42c336d6 100644 --- a/signal.c +++ b/signal.c @@ -119,23 +119,20 @@ evsignal_init(struct event_base *base) base->sig.ev_signal.ev_flags |= EVLIST_INTERNAL; } +/* Helper: set the signal handler for evsignal to handler in base, so that + * we can restore the original handler when we clear the current one. */ int -evsignal_add(struct event *ev) +_evsignal_set_handler(struct event_base *base, + int evsignal, void (*handler)(int)) { - int evsignal; #ifdef HAVE_SIGACTION struct sigaction sa; #else ev_sighandler_t sh; #endif - struct event_base *base = ev->ev_base; - struct evsignal_info *sig = &ev->ev_base->sig; + struct evsignal_info *sig = &base->sig; void *p; - if (ev->ev_events & (EV_READ|EV_WRITE)) - event_errx(1, "%s: EV_SIGNAL incompatible use", __func__); - evsignal = EVENT_SIGNAL(ev); - /* * resize saved signal handler array up to the highest signal number. * a dynamic array is used to keep footprint on the low side. @@ -160,10 +157,9 @@ evsignal_add(struct event *ev) } /* save previous handler and setup new handler */ - event_debug(("%s: %p: changing signal handler", __func__, ev)); #ifdef HAVE_SIGACTION memset(&sa, 0, sizeof(sa)); - sa.sa_handler = evsignal_handler; + sa.sa_handler = handler; sa.sa_flags |= SA_RESTART; sigfillset(&sa.sa_mask); @@ -173,13 +169,32 @@ evsignal_add(struct event *ev) return (-1); } #else - if ((sh = signal(evsignal, evsignal_handler)) == SIG_ERR) { + if ((sh = signal(evsignal, handler)) == SIG_ERR) { event_warn("signal"); free(sig->sh_old[evsignal]); return (-1); } *sig->sh_old[evsignal] = sh; #endif + + return (0); +} + +int +evsignal_add(struct event *ev) +{ + int evsignal; + struct event_base *base = ev->ev_base; + struct evsignal_info *sig = &ev->ev_base->sig; + + if (ev->ev_events & (EV_READ|EV_WRITE)) + event_errx(1, "%s: EV_SIGNAL incompatible use", __func__); + evsignal = EVENT_SIGNAL(ev); + + 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; @@ -192,21 +207,17 @@ evsignal_add(struct event *ev) } int -evsignal_del(struct event *ev) +_evsignal_restore_handler(struct event_base *base, int evsignal) { - int evsignal, ret = 0; - struct event_base *base = ev->ev_base; - struct evsignal_info *sig = &ev->ev_base->sig; + int ret = 0; + struct evsignal_info *sig = &base->sig; #ifdef HAVE_SIGACTION struct sigaction *sh; #else ev_sighandler_t *sh; #endif - evsignal = EVENT_SIGNAL(ev); - /* restore previous handler */ - event_debug(("%s: %p: restoring signal handler", __func__, ev)); sh = sig->sh_old[evsignal]; sig->sh_old[evsignal] = NULL; #ifdef HAVE_SIGACTION @@ -225,6 +236,13 @@ evsignal_del(struct event *ev) return ret; } +int +evsignal_del(struct event *ev) +{ + event_debug(("%s: %p: restoring signal handler", __func__, ev)); + return _evsignal_restore_handler(ev->ev_base, EVENT_SIGNAL(ev)); +} + static void evsignal_handler(int sig) { diff --git a/test/regress.c b/test/regress.c index 427b5c34..d4ebab96 100644 --- a/test/regress.c +++ b/test/regress.c @@ -526,18 +526,19 @@ test_signal_pipeloss(void) /* * make two bases to catch signals, use both of them. this only works * for event mechanisms that use our signal pipe trick. kqueue handles - * signals internally, and it looks like the first kqueue always gets the - * signal. + * signals internally, and all interested kqueues get all the signals. */ void test_signal_switchbase(void) { struct event ev1, ev2; struct event_base *base1, *base2; + int is_kqueue; test_ok = 0; printf("Signal switchbase: "); base1 = event_init(); base2 = event_init(); + is_kqueue = !strcmp(event_get_method(),"kqueue"); signal_set(&ev1, SIGUSR1, signal_cb, &ev1); signal_set(&ev2, SIGUSR1, signal_cb, &ev2); if (event_base_set(base1, &ev1) || @@ -552,15 +553,22 @@ test_signal_switchbase(void) /* can handle signal before loop is called */ raise(SIGUSR1); event_base_loop(base2, EVLOOP_NONBLOCK); + if (is_kqueue) { + if (!test_ok) + goto done; + test_ok = 0; + } event_base_loop(base1, EVLOOP_NONBLOCK); - if (test_ok) { + if (test_ok && !is_kqueue) { test_ok = 0; + /* set base1 to handle signals */ event_base_loop(base1, EVLOOP_NONBLOCK); raise(SIGUSR1); event_base_loop(base1, EVLOOP_NONBLOCK); event_base_loop(base2, EVLOOP_NONBLOCK); } + done: event_base_free(base1); event_base_free(base2); cleanup_test();