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.
This commit is contained in:
Azat Khuzhin 2015-01-03 19:37:15 +03:00
parent 0b49ae3459
commit 92a359ee3a

View File

@ -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 */