be_pair: release shared lock with the latest of bufferevent_pair

Then next code sample will use free'd lock:
  evthread_use_pthreads();
  ...
  assert(!bufferevent_pair_new(base, BEV_OPT_THREADSAFE, pair));
  ...
  bufferevent_free(pair[0]); # refcnt == 0 -> unlink
  bufferevent_free(pair[1]); # refcnt == 0 -> unlink
  ...
  event_base_free() -> finalizers -> EVTHREAD_FREE_LOCK(bev1->lock)
                                  -> BEV_LOCK(bev2->lock) <-- *already freed*

While if you will reverse the order:
  bufferevent_free(pair[1]); # refcnt == 0 -> unlink
  bufferevent_free(pair[0]); # refcnt == 0 -> unlink
  ...
  event_base_free() -> finalizers -> BEV_LOCK(bev2->lock)/!own_lock/BEV_UNLOCK(bev2->lock)
                                  -> EVTHREAD_FREE_LOCK(bev1->lock) (own_lock)

It is ok now, but I guess that it will be better to relax order of
freeing pairs.

diff --git a/bufferevent_pair.c b/bufferevent_pair.c
index 4340f23723c0bf8dc63d6b3e5e1d741edfd99111..8154e17be732dcb7a67f8efa3e51a2ac4d1e4440 100644 (file)
--- a/bufferevent_pair.c
+++ b/bufferevent_pair.c
@@ -45,6 +45,8 @@
 struct bufferevent_pair {
        struct bufferevent_private bev;
        struct bufferevent_pair *partner;
+       /* For ->destruct() lock checking */
+       struct bufferevent_pair *unlinked_partner;
 };
 
 
@@ -265,11 +267,40 @@ be_pair_unlink(struct bufferevent *bev)
        struct bufferevent_pair *bev_p = upcast(bev);
 
        if (bev_p->partner) {
+               bev_p->unlinked_partner = bev_p->partner;
                bev_p->partner->partner = NULL;
                bev_p->partner = NULL;
        }
 }
 
+/* Free *shared* lock in the latest be (since we share it between two of them). */
+static void
+be_pair_destruct(struct bufferevent *bev)
+{
+       struct bufferevent_pair *bev_p = upcast(bev);
+
+       /* Transfer ownership of the lock into partner, otherwise we will use
+        * already free'd lock during freeing second bev, see next example:
+        *
+        * bev1->own_lock = 1
+        * bev2->own_lock = 0
+        * bev2->lock = bev1->lock
+        *
+        * bufferevent_free(bev1) # refcnt == 0 -> unlink
+        * bufferevent_free(bev2) # refcnt == 0 -> unlink
+        *
+        * event_base_free() -> finilizers -> EVTHREAD_FREE_LOCK(bev1->lock)
+        *                                 -> BEV_LOCK(bev2->lock) <-- already freed
+        *
+        * Where bev1 == pair[0], bev2 == pair[1].
+        */
+       if (bev_p->unlinked_partner && bev_p->bev.own_lock) {
+               bev_p->unlinked_partner->bev.own_lock = 1;
+               bev_p->bev.own_lock = 0;
+       }
+       bev_p->unlinked_partner = NULL;
+}
+
 static int
 be_pair_flush(struct bufferevent *bev, short iotype,
     enum bufferevent_flush_mode mode)
@@ -320,7 +351,7 @@ const struct bufferevent_ops bufferevent_ops_pair = {
        be_pair_enable,
        be_pair_disable,
        be_pair_unlink,
-       NULL, /* be_pair_destruct, */
+       be_pair_destruct,
        bufferevent_generic_adj_timeouts_,
        be_pair_flush,
        NULL, /* ctrl */