Release locks on bufferevents while executing callbacks

This fixes a dead lock for me where bufferevents in different event
loops use each other and access their input/output buffers (proxy-like
scenario).

diff --git a/bufferevent.c b/bufferevent.c
index 1b6df74efdb675e6dbd5aaf8329209d152bab713..75ff9b72cf9ab44fdc2532caf3a8af8cdfb9e16d 100644 (file)
--- a/bufferevent.c
+++ b/bufferevent.c
@@ -127,13 +127,11 @@ bufferevent_inbuf_wm_cb(struct evbuffer *buf,
 }
 
 static void
-bufferevent_run_deferred_callbacks(struct deferred_cb *_, void *arg)
+bufferevent_run_deferred_callbacks_locked(struct deferred_cb *_, void *arg)
 {
        struct bufferevent_private *bufev_private = arg;
        struct bufferevent *bufev = &bufev_private->bev;
 
-       /* XXXX It would be better to run these without holding the
-        * bufferevent lock */
        BEV_LOCK(bufev);
        if ((bufev_private->eventcb_pending & BEV_EVENT_CONNECTED) &&
            bufev->errorcb) {
@@ -161,6 +159,51 @@ bufferevent_run_deferred_callbacks(struct deferred_cb *_, void *arg)
        _bufferevent_decref_and_unlock(bufev);
 }
 
+static void
+bufferevent_run_deferred_callbacks_unlocked(struct deferred_cb *_, void *arg)
+{
+       struct bufferevent_private *bufev_private = arg;
+       struct bufferevent *bufev = &bufev_private->bev;
+
+       BEV_LOCK(bufev);
+#define UNLOCKED(stmt) \
+       do { BEV_UNLOCK(bufev); stmt; BEV_LOCK(bufev); } while(0)
+
+       if ((bufev_private->eventcb_pending & BEV_EVENT_CONNECTED) &&
+           bufev->errorcb) {
+               /* The "connected" happened before any reads or writes, so
+                  send it first. */
+               bufferevent_event_cb errorcb = bufev->errorcb;
+               void *cbarg = bufev->cbarg;
+               bufev_private->eventcb_pending &= ~BEV_EVENT_CONNECTED;
+               UNLOCKED(errorcb(bufev, BEV_EVENT_CONNECTED, cbarg));
+       }
+       if (bufev_private->readcb_pending && bufev->readcb) {
+               bufferevent_data_cb readcb = bufev->readcb;
+               void *cbarg = bufev->cbarg;
+               bufev_private->readcb_pending = 0;
+               UNLOCKED(readcb(bufev, cbarg));
+       }
+       if (bufev_private->writecb_pending && bufev->writecb) {
+               bufferevent_data_cb writecb = bufev->writecb;
+               void *cbarg = bufev->cbarg;
+               bufev_private->writecb_pending = 0;
+               UNLOCKED(writecb(bufev, cbarg));
+       }
+       if (bufev_private->eventcb_pending && bufev->errorcb) {
+               bufferevent_event_cb errorcb = bufev->errorcb;
+               void *cbarg = bufev->cbarg;
+               short what = bufev_private->eventcb_pending;
+               int err = bufev_private->errno_pending;
+               bufev_private->eventcb_pending = 0;
+               bufev_private->errno_pending = 0;
+               EVUTIL_SET_SOCKET_ERROR(err);
+               UNLOCKED(errorcb(bufev,what,cbarg));
+       }
+       _bufferevent_decref_and_unlock(bufev);
+#undef UNLOCKED
+}
+
 #define SCHEDULE_DEFERRED(bevp)                                                \
        do {                                                            \
                event_deferred_cb_schedule(                             \
@@ -275,10 +318,20 @@ bufferevent_init_common(struct bufferevent_private *bufev_private,
                }
        }
 #endif
+       if ((options & (BEV_OPT_DEFER_CALLBACKS|BEV_OPT_UNLOCK_CALLBACKS))
+           == BEV_OPT_UNLOCK_CALLBACKS) {
+               event_warnx("UNLOCK_CALLBACKS requires DEFER_CALLBACKS");
+               return -1;
+       }
        if (options & BEV_OPT_DEFER_CALLBACKS) {
-               event_deferred_cb_init(&bufev_private->deferred,
-                   bufferevent_run_deferred_callbacks,
-                   bufev_private);
+               if (options & BEV_OPT_UNLOCK_CALLBACKS)
+                       event_deferred_cb_init(&bufev_private->deferred,
+                           bufferevent_run_deferred_callbacks_unlocked,
+                           bufev_private);
+               else
+                       event_deferred_cb_init(&bufev_private->deferred,
+                           bufferevent_run_deferred_callbacks_locked,
+                           bufev_private);
        }
 
        bufev_private->options = options;

diff --git a/include/event2/bufferevent.h b/include/event2/bufferevent.h
index b1d7e43a01958e5e0af8b492b68b9c0d6115192d..a0c9d390f5f8ede601295a8ac29bdf78f0dcb5b1 100644 (file)
--- a/include/event2/bufferevent.h
+++ b/include/event2/bufferevent.h
@@ -119,7 +119,13 @@ enum bufferevent_options {
        BEV_OPT_THREADSAFE = (1<<1),
 
        /** If set, callbacks are run deferred in the event loop. */
-       BEV_OPT_DEFER_CALLBACKS = (1<<2)
+       BEV_OPT_DEFER_CALLBACKS = (1<<2),
+
+       /** If set, callbacks are executed without locks being held on the
+       * bufferevent.  This option currently requires that
+       * BEV_OPT_DEFER_CALLBACKS also be set; a future version of Libevent
+       * might remove the requirement.*/
+       BEV_OPT_UNLOCK_CALLBACKS = (1<<3)
 };
 
 /**