Detect and refuse reentrant event_base_loop() calls

Calling event_base_loop on a base from inside a callback invoked by
that same base, or from two threads at once, has long been a way to
get exceedingly hard-to-diagnose errors.  This patch adds code to
detect such reentrant invocatinos, and exit quickly with a warning
that should explain what went wrong.

diff --git a/event-internal.h b/event-internal.h
index 0da708663b674745093aef1f3f41e0d768ce938b..f9de18ad3f3837ed20d1930513ac44ec4918e713 100644 (file)
--- a/event-internal.h
+++ b/event-internal.h
@@ -190,6 +190,10 @@ struct event_base {
        /** Set if we should terminate the loop immediately */
        int event_break;
 
+       /** Set if we're running the event_base_loop function, to prevent
+        * reentrant invocation. */
+       int running_loop;
+
        /* Active event management. */
        /** An array of nactivequeues queues for active events (ones that
         * have triggered, and whose callbacks need to be called).  Low

diff --git a/event.c b/event.c
index 469da7e651e5f0baa9bd6e22f4e298c7f146c5f9..7538b5e2694e0505a21b8847e16d28f7b816e2e2 100644 (file)
--- a/event.c
+++ b/event.c
@@ -1399,6 +1399,15 @@ event_base_loop(struct event_base *base, int flags)
         * as we invoke user callbacks. */
        EVBASE_ACQUIRE_LOCK(base, th_base_lock);
 
+       if (base->running_loop) {
+               event_warn("%s: reentrant invocation.  Only one event_base_loop"
+                   " can run on each event_base at once.", __func__);
+               EVBASE_RELEASE_LOCK(base, th_base_lock);
+               return -1;
+       }
+
+       base->running_loop = 1;
+
        clear_time_cache(base);
 
        if (base->sig.ev_signal_added)
@@ -1470,6 +1479,7 @@ event_base_loop(struct event_base *base, int flags)
 
 done:
        clear_time_cache(base);
+       base->running_loop = 0;
 
        EVBASE_RELEASE_LOCK(base, th_base_lock);
 

diff --git a/test/regress.c b/test/regress.c
index ae932cc9935fcdfc2f402e543af5537af676dd0a..5ad1342ea8aa5379ccf738f5a483c7899fd762fe 100644 (file)
--- a/test/regress.c
+++ b/test/regress.c
@@ -1156,6 +1156,40 @@ end:
        ;
 }
 
+static int reentrant_cb_run = 0;
+
+static void
+bad_reentrant_run_loop_cb(evutil_socket_t fd, short what, void *ptr)
+{
+       struct event_base *base = ptr;
+       int r;
+       reentrant_cb_run = 1;
+       /* This reentrant call to event_base_loop should be detected and
+        * should fail */
+       r = event_base_loop(base, 0);
+       tt_int_op(r, ==, -1);
+end:
+       ;
+}
+
+static void
+test_bad_reentrant(void *ptr)
+{
+       struct basic_test_data *data = ptr;
+       struct event_base *base = data->base;
+       struct event ev;
+       int r;
+       event_assign(&ev, base, -1,
+           0, bad_reentrant_run_loop_cb, base);
+
+       event_active(&ev, EV_WRITE, 1);
+       r = event_base_loop(base, 0);
+       tt_int_op(r, ==, 1);
+       tt_int_op(reentrant_cb_run, ==, 1);
+end:
+       ;
+}
+
 static void
 test_event_base_new(void *ptr)
 {
@@ -2072,6 +2106,7 @@ struct testcase_t main_testcases[] = {
        BASIC(manipulate_active_events, TT_FORK|TT_NEED_BASE),
 
        BASIC(bad_assign, TT_FORK|TT_NEED_BASE|TT_NO_LOGS),
+       BASIC(bad_reentrant, TT_FORK|TT_NEED_BASE|TT_NO_LOGS),
 
        /* These are still using the old API */
        LEGACY(persistent_timeout, TT_FORK|TT_NEED_BASE),