r14944@tombo: nickm | 2007-11-25 12:12:28 -0500

 Make kqueue pass more unit tests.

svn:r544

diff --git a/ChangeLog b/ChangeLog
index 7813d2afafae0f499b9fe87917fb32544e7bbf4a..cc698f470c26a41b582ae9efe6c76c61327193f9 100644 (file)
--- 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 d8e984d577fb97cfe209bd7392ea058aa1d18332..424ea3e913ad95968f5e291924177accd4475269 100644 (file)
--- 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 8f92c2cdb0d85df7a42d71809dd15e570f1bf5ee..1f2e35148e40b59b2c521c3b30d938ae562fe505 100644 (file)
--- 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 9388e98ac482f0d843d68ad5804ecec37d0d0488..60caef309e966e7c67d3e244abbe38daffc8f436 100644 (file)
--- 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 111d06283c13454c8f5ad354bd851bffe4517a7a..3e5378a0d4e554fb5e302f243861bf3a0a7e0b19 100644 (file)
--- 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();