From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 10 Jul 2009 19:34:00 +0000 (+0000)
Subject: Add a lock/unlock pair inside the event callbacks in bufferevents.
X-Git-Tag: release-2.0.3-alpha~190
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a501d6833bae3415381e62acd6e45f4a22fea5b6;p=libevent

Add a lock/unlock pair inside the event callbacks in bufferevents.

This fixes part of bug 2800642, I believe, though there is still a
general race condition in multithreaded use of events that we need to
think about.

svn:r1337
---

diff --git a/ChangeLog b/ChangeLog
index bfebd19f..8a38b283 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -40,6 +40,7 @@ Changes in 2.0.2-alpha:
  o Fix preamble of rpcgen-generated files to rely on event2 includes; based on work by jmansion; patch from Zack Weinberg.
  o Allow specifying the output filename for rpcgen; based on work by jmansion; patch from Zack Weinberg.
  o Allow C identifiers as struct names; allow multiple comments in .rpc files; from Zack Weinberg
+ o Mitigate a race condition when using socket bufferevents in multiple threads.
 
 
 Changes in 2.0.1-alpha:
diff --git a/bufferevent.c b/bufferevent.c
index be1630e3..971ded7e 100644
--- a/bufferevent.c
+++ b/bufferevent.c
@@ -574,13 +574,17 @@ static void
 bufferevent_generic_read_timeout_cb(evutil_socket_t fd, short event, void *ctx)
 {
 	struct bufferevent *bev = ctx;
+	BEV_LOCK(bev);
 	_bufferevent_run_eventcb(bev, BEV_EVENT_TIMEOUT|BEV_EVENT_READING);
+	BEV_UNLOCK(bev);
 }
 static void
 bufferevent_generic_write_timeout_cb(evutil_socket_t fd, short event, void *ctx)
 {
 	struct bufferevent *bev = ctx;
+	BEV_LOCK(bev);
 	_bufferevent_run_eventcb(bev, BEV_EVENT_TIMEOUT|BEV_EVENT_WRITING);
+	BEV_UNLOCK(bev);
 }
 
 void
diff --git a/bufferevent_sock.c b/bufferevent_sock.c
index e8e5f473..e4b0836f 100644
--- a/bufferevent_sock.c
+++ b/bufferevent_sock.c
@@ -122,6 +122,8 @@ bufferevent_readcb(evutil_socket_t fd, short event, void *arg)
 	short what = BEV_EVENT_READING;
 	int howmuch = -1;
 
+	BEV_LOCK(arg);
+
 	if (event == EV_TIMEOUT) {
 		what |= BEV_EVENT_TIMEOUT;
 		goto error;
@@ -138,7 +140,7 @@ bufferevent_readcb(evutil_socket_t fd, short event, void *arg)
 		/* we somehow lowered the watermark, stop reading */
 		if (howmuch <= 0) {
 			bufferevent_wm_suspend_read(bufev);
-			return;
+			goto done;
 		}
 	}
 
@@ -166,14 +168,17 @@ bufferevent_readcb(evutil_socket_t fd, short event, void *arg)
             bufev->readcb != NULL)
 		_bufferevent_run_readcb(bufev);
 
-	return;
+	goto done;
 
  reschedule:
-	return;
+	goto done;
 
  error:
 	event_del(&bufev->ev_read);
 	_bufferevent_run_eventcb(bufev, what);
+
+ done:
+	BEV_UNLOCK(bufev);
 }
 
 static void
@@ -185,6 +190,8 @@ bufferevent_writecb(evutil_socket_t fd, short event, void *arg)
 	int res = 0;
 	short what = BEV_EVENT_WRITING;
 
+	BEV_LOCK(bufev);
+
 	if (event == EV_TIMEOUT) {
 		what |= BEV_EVENT_TIMEOUT;
 		goto error;
@@ -194,7 +201,7 @@ bufferevent_writecb(evutil_socket_t fd, short event, void *arg)
 		_bufferevent_run_eventcb(bufev, BEV_EVENT_CONNECTED);
 		if (!(bufev->enabled & EV_WRITE)) {
 			event_del(&bufev->ev_write);
-			return;
+			goto done;
 		}
 	}
 
@@ -226,16 +233,19 @@ bufferevent_writecb(evutil_socket_t fd, short event, void *arg)
 	    evbuffer_get_length(bufev->output) <= bufev->wm_write.low)
 		_bufferevent_run_writecb(bufev);
 
-	return;
+	goto done;
 
  reschedule:
 	if (evbuffer_get_length(bufev->output) == 0)
 		event_del(&bufev->ev_write);
-	return;
+	goto done;
 
  error:
 	event_del(&bufev->ev_write);
 	_bufferevent_run_eventcb(bufev, what);
+
+ done:
+	BEV_UNLOCK(bufev);
 }
 
 struct bufferevent *