Skip to content

Commit

Permalink
Fix a warn-and-fail bug in kqueue by providing kevent() room to repor…
Browse files Browse the repository at this point in the history
…t 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.
  • Loading branch information
nmathewson committed May 3, 2011
1 parent 9556a7d commit 28317a0
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 15 deletions.
46 changes: 37 additions & 9 deletions kqueue.c
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
19 changes: 13 additions & 6 deletions test/regress.c
Expand Up @@ -2204,14 +2204,15 @@ 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. */
#define MANY 70

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];
Expand All @@ -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:
Expand Down Expand Up @@ -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 },

Expand Down

0 comments on commit 28317a0

Please sign in to comment.