|
From: Nick M. <ni...@us...> - 2011-05-03 18:14:52
|
Author: Nick Mathewson <ni...@to...>
Date: Tue, 3 May 2011 13:54:57 -0400
Subject: Fix a warn-and-fail bug in kqueue by providing kevent() room to report errors
Commit: 28317a087e58dceff85a448c22bbbafebed0b6ab
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.
---
kqueue.c | 46 +++++++++++++++++++++++++++++++++++++---------
test/regress.c | 19 +++++++++++++------
2 files changed, 50 insertions(+), 15 deletions(-)
diff --git a/kqueue.c b/kqueue.c
index 9201298..8b48634 100644
--- a/kqueue.c
+++ b/kqueue.c
@@ -229,6 +229,23 @@ kq_build_changes_list(const struct event_changelist *changelist,
}
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)
{
struct kqop *kqop = base->evbase;
@@ -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 ee6962f..ec7580e 100644
--- 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 },
--
1.7.0.1
|