From b557b175c00dc462c1fce25f6e7dd67121d2c001 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Sun, 21 Mar 2010 13:28:48 -0400
Subject: [PATCH] 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.
---
 event-internal.h |  4 ++++
 event.c          | 10 ++++++++++
 test/regress.c   | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+)

diff --git a/event-internal.h b/event-internal.h
index 0da70866..f9de18ad 100644
--- 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 469da7e6..7538b5e2 100644
--- 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 ae932cc9..5ad1342e 100644
--- 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),
-- 
2.40.0