Fix a warn-and-fail bug in kqueue by providing kevent() room to report errors

Apparently, kevent fails gracefully if there is not enough space in its
output events array to report every _event_... but it just dies and returns
-1 if there is not enough space to report every _error_.

There are a couple of possible fixes here.  One would to handle -1
returns from kevent better by re-growing the array and retrying... but
that seems a little error prone.  Instead, I'm just going to say that
the events array must be large enough to handle all the errors.

This patch also adds a unit test designed to make sure that our
many-events-out code works even if not all the events are added at
once.

diff --git a/kqueue.c b/kqueue.c
index 920129826f46a6dbe009c3c6d095f8e564e69bee..8b4863465ad2e10be4843944780702053bc2f0d5 100644 (file)
--- a/kqueue.c
+++ b/kqueue.c
@@ -228,6 +228,23 @@ kq_build_changes_list(const struct event_changelist *changelist,
        return n_changes;
 }
 
+static int
+kq_grow_events(struct kqop *kqop, size_t new_size)
+{
+       struct kevent *newresult;
+
+       newresult = mm_realloc(kqop->events,
+           new_size * sizeof(struct kevent));
+
+       if (newresult) {
+               kqop->events = newresult;
+               kqop->events_size = new_size;
+               return 0;
+       } else {
+               return -1;
+       }
+}
+
 static int
 kq_dispatch(struct event_base *base, struct timeval *tv)
 {
@@ -255,6 +272,25 @@ kq_dispatch(struct event_base *base, struct timeval *tv)
        changes = kqop->changes;
        kqop->changes = NULL;
 
+       /* Make sure that 'events' is at least as long as the list of changes:
+        * otherwise errors in the changes can get reported as a -1 return
+        * value from kevent() rather than as EV_ERROR events in the events
+        * array.
+        *
+        * (We could instead handle -1 return values from kevent() by
+        * retrying with a smaller changes array or a larger events array,
+        * but this approach seems less risky for now.)
+        */
+       if (kqop->events_size < n_changes) {
+               int new_size = kqop->events_size;
+               do {
+                       new_size *= 2;
+               } while (new_size < n_changes);
+
+               kq_grow_events(kqop, new_size);
+               events = kqop->events;
+       }
+
        EVBASE_RELEASE_LOCK(base, th_base_lock);
 
        res = kevent(kqop->kq, changes, n_changes,
@@ -319,17 +355,9 @@ kq_dispatch(struct event_base *base, struct timeval *tv)
        }
 
        if (res == kqop->events_size) {
-               struct kevent *newresult;
-               int size = kqop->events_size;
                /* We used all the events space that we have. Maybe we should
                   make it bigger. */
-               size *= 2;
-               newresult = mm_realloc(kqop->events,
-                   size * sizeof(struct kevent));
-               if (newresult) {
-                       kqop->events = newresult;
-                       kqop->events_size = size;
-               }
+               kq_grow_events(kqop, kqop->events_size * 2);
        }
 
        return (0);

diff --git a/test/regress.c b/test/regress.c
index ee6962fcca7ab27c30769e9721c3054afd9c8217..ec7580ed6d0c1a89a9dd9da13255324edcccf2cc 100644 (file)
--- a/test/regress.c
+++ b/test/regress.c
@@ -2204,7 +2204,7 @@ many_event_cb(evutil_socket_t fd, short event, void *arg)
 static void
 test_many_events(void *arg)
 {
-       /* Try 70 events that should all be aready at once.  This will
+       /* Try 70 events that should all be ready at once.  This will
         * exercise the "resize" code on most of the backends, and will make
         * sure that we can get past the 64-handle limit of some windows
         * functions. */
@@ -2212,6 +2212,7 @@ test_many_events(void *arg)
 
        struct basic_test_data *data = arg;
        struct event_base *base = data->base;
+       int one_at_a_time = data->setup_data != NULL;
        evutil_socket_t sock[MANY];
        struct event *ev[MANY];
        int called[MANY];
@@ -2228,15 +2229,20 @@ test_many_events(void *arg)
                sock[i] = socket(AF_INET, SOCK_DGRAM, 0);
                tt_assert(sock[i] >= 0);
                called[i] = 0;
-               ev[i] = event_new(base, sock[i], EV_WRITE, many_event_cb,
-                   &called[i]);
+               ev[i] = event_new(base, sock[i], EV_WRITE|EV_PERSIST,
+                   many_event_cb, &called[i]);
                event_add(ev[i], NULL);
+               if (one_at_a_time)
+                       event_base_loop(base, EVLOOP_NONBLOCK|EVLOOP_ONCE);
        }
 
-       event_base_loop(base, EVLOOP_NONBLOCK);
+       event_base_loop(base, EVLOOP_NONBLOCK|EVLOOP_ONCE);
 
        for (i = 0; i < MANY; ++i) {
-               tt_int_op(called[i], ==, 1);
+               if (one_at_a_time)
+                       tt_int_op(called[i], ==, MANY - i + 1);
+               else
+                       tt_int_op(called[i], ==, 1);
        }
 
 end:
@@ -2303,7 +2309,8 @@ struct testcase_t main_testcases[] = {
        { "dup_fd", test_dup_fd, TT_ISOLATED, &basic_setup, NULL },
 #endif
        { "mm_functions", test_mm_functions, TT_FORK, NULL, NULL },
-       BASIC(many_events, TT_ISOLATED),
+       { "many_events", test_many_events, TT_ISOLATED, &basic_setup, NULL },
+       { "many_events_slow_add", test_many_events, TT_ISOLATED, &basic_setup, (void*)1 },
 
        { "struct_event_size", test_struct_event_size, 0, NULL, NULL },