From 2823cb057927110719a7203c2346d6be8aa8468f Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Sun, 25 Nov 2007 17:15:28 +0000 Subject: [PATCH] r14944@tombo: nickm | 2007-11-25 12:12:28 -0500 Make kqueue pass more unit tests. svn:r544 --- ChangeLog | 3 ++- event-internal.h | 4 ++++ kqueue.c | 14 +++++++----- signal.c | 55 ++++++++++++++++++++++++++++++++---------------- test/regress.c | 14 +++++++++--- 5 files changed, 63 insertions(+), 27 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7813d2af..cc698f47 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,7 +7,8 @@ Changes in current version: o New function, event_{base_}loopbreak. Like event_loopexit, it makes an event loop stop executing and return. Unlike event_loopexit, it keeps subsequent pending events from getting executed. Patch from Scott Lamb o provide event_reinit() to reintialize an event_base after fork o New function event_set_mem_functinons. It allows the user to give libevent replacement functions to use for memory management in place of malloc(), free(), etc. This should be generally useful for memory instrumentation, specialized allocators, and so on. - + o The kqueue implementation now catches signals that are raised after event_add() is called but before the event_loop() call. This makes it match the other implementations. + o The kqueue implementation now restores original signal handlers correctly when its signal events are removed. Changes in 1.4.0: diff --git a/event-internal.h b/event-internal.h index d8e984d5..424ea3e9 100644 --- a/event-internal.h +++ b/event-internal.h @@ -87,6 +87,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 8f92c2cd..1f2e3514 100644 --- a/kqueue.c +++ b/kqueue.c @@ -296,6 +296,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; @@ -304,11 +305,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; @@ -372,7 +376,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 9388e98a..60caef30 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"); event_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 @@ -220,11 +231,19 @@ evsignal_del(struct event *ev) ret = -1; } #endif + event_free(sh); 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 111d0628..3e5378a0 100644 --- a/test/regress.c +++ b/test/regress.c @@ -568,18 +568,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) || @@ -594,15 +595,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(); -- 2.40.0