]> granicus.if.org Git - libevent/commitdiff
Fix race caused by event_active
authorvjpai <vpai@google.com>
Mon, 22 Sep 2014 19:19:37 +0000 (12:19 -0700)
committerNick Mathewson <nickm@torproject.org>
Mon, 1 Dec 2014 00:24:15 +0000 (19:24 -0500)
There is a race between manual event_active and natural event activation. If both happen at the same time on the same FD, they would both be protected by the same event base lock except for 1 LoC where the fields of struct event are read without any kind of lock. This commit does those reads into local variables inside the lock and then invokes the callback with those local arguments outside the lock. In 2.0-stable, none of this is inside the lock; in HEAD, only the callback is read inside the lock. This gets the callback and all 3 arguments inside the lock before calling it outside the lock.

event.c

diff --git a/event.c b/event.c
index a979f1f2649695678a1b152368baa6f5b14b35e4..fab419afe97d16b6bb77f846768a7ada9cbda7cd 100644 (file)
--- a/event.c
+++ b/event.c
@@ -1256,6 +1256,14 @@ done:
 static inline void
 event_persist_closure(struct event_base *base, struct event *ev)
 {
+       // Define our callback, we use this to store our callback before it's executed
+       void (*evcb_callback)(evutil_socket_t, short, void *);
+
+        // Other fields of *ev that must be stored before executing
+        evutil_socket_t evcb_fd;
+        short evcb_res;
+        void *evcb_arg;
+
        /* reschedule the persistent event if we have a timeout. */
        if (ev->ev_io_timeout.tv_sec || ev->ev_io_timeout.tv_usec) {
                /* If there was a timeout, we want it to run at an interval of
@@ -1297,8 +1305,18 @@ event_persist_closure(struct event_base *base, struct event *ev)
                run_at.tv_usec |= usec_mask;
                event_add_internal(ev, &run_at, 1);
        }
-       EVBASE_RELEASE_LOCK(base, th_base_lock);
-       (*ev->ev_callback)(ev->ev_fd, ev->ev_res, ev->ev_arg);
+
+       // Save our callback before we release the lock
+       evcb_callback = ev->ev_callback;
+        evcb_fd = ev->ev_fd;
+        evcb_res = ev->ev_res;
+        evcb_arg = ev->ev_arg;
+
+       // Release the lock
+       EVBASE_RELEASE_LOCK(base, th_base_lock);
+
+       // Execute the callback
+        (evcb_callback)(evcb_fd, evcb_res, evcb_arg);
 }
 
 /*