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 bfebd19ff9a522048a2d8eb1238ed0a35fb42395..8a38b2837ea38eb4d0d97b30fa6140d9dcd23f86 100644 (file)
--- 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 be1630e340893c6af33535250ec22dc6afdbb2aa..971ded7eeca156ca7b87056f41de3042b8454be8 100644 (file)
--- 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 e8e5f4736e6f0a682ee463c3248686e72dc961c5..e4b0836fca90bf9de2b831a00a6b69ec90054ca0 100644 (file)
--- 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 *