Use finalization feature so bufferevents can avoid deadlocks

Since the bufferevents' events are now EV_FINALIZE (name pending),
they won't deadlock.  To clean up properly, though, we must use the
finalization feature.

This patch also split bufferevent deallocation into an "unlink" step
that happens fast, and a "destruct" step that happens after
finalization.

More work is needed: there needs to be a way to specify a finalizer
for the bufferevent's argument itself.  Also, this finalizer business
makes lots of the reference counting we were doing unnecessary.

Also, more testing is needed.

1 files changed, 32 insertions, 21 deletions

diff --git a/bufferevent_openssl.c b/bufferevent_openssl.c
index 99ed5f8d..48c61c08 100644
--- a/bufferevent_openssl.c
+++ b/bufferevent_openssl.c
@@ -326,6 +326,7 @@ struct bufferevent_openssl {
 
 static int be_openssl_enable(struct bufferevent *, short);
 static int be_openssl_disable(struct bufferevent *, short);
+static void be_openssl_unlink(struct bufferevent *);
 static void be_openssl_destruct(struct bufferevent *);
 static int be_openssl_adj_timeouts(struct bufferevent *);
 static int be_openssl_flush(struct bufferevent *bufev,
@@ -337,6 +338,7 @@ const struct bufferevent_ops bufferevent_ops_openssl = {
 	evutil_offsetof(struct bufferevent_openssl, bev.bev),
 	be_openssl_enable,
 	be_openssl_disable,
+	be_openssl_unlink,
 	be_openssl_destruct,
 	be_openssl_adj_timeouts,
 	be_openssl_flush,
@@ -977,9 +979,11 @@ set_open_callbacks(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd)
 			event_del(&bev->ev_write);
 		}
 		event_assign(&bev->ev_read, bev->ev_base, fd,
-		    EV_READ|EV_PERSIST, be_openssl_readeventcb, bev_ssl);
+		    EV_READ|EV_PERSIST|EV_FINALIZE,
+		    be_openssl_readeventcb, bev_ssl);
 		event_assign(&bev->ev_write, bev->ev_base, fd,
-		    EV_WRITE|EV_PERSIST, be_openssl_writeeventcb, bev_ssl);
+		    EV_WRITE|EV_PERSIST|EV_FINALIZE,
+		    be_openssl_writeeventcb, bev_ssl);
 		if (rpending)
 			r1 = bufferevent_add_event_(&bev->ev_read, &bev->timeout_read);
 		if (wpending)
@@ -1079,9 +1083,11 @@ set_handshake_callbacks(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd)
 			event_del(&bev->ev_write);
 		}
 		event_assign(&bev->ev_read, bev->ev_base, fd,
-		    EV_READ|EV_PERSIST, be_openssl_handshakeeventcb, bev_ssl);
+		    EV_READ|EV_PERSIST|EV_FINALIZE,
+		    be_openssl_handshakeeventcb, bev_ssl);
 		event_assign(&bev->ev_write, bev->ev_base, fd,
-		    EV_WRITE|EV_PERSIST, be_openssl_handshakeeventcb, bev_ssl);
+		    EV_WRITE|EV_PERSIST|EV_FINALIZE,
+		    be_openssl_handshakeeventcb, bev_ssl);
 		if (fd >= 0) {
 			r1 = bufferevent_add_event_(&bev->ev_read, &bev->timeout_read);
 			r2 = bufferevent_add_event_(&bev->ev_write, &bev->timeout_write);
@@ -1176,17 +1182,10 @@ be_openssl_disable(struct bufferevent *bev, short events)
 }
 
 static void
-be_openssl_destruct(struct bufferevent *bev)
+be_openssl_unlink(struct bufferevent *bev)
 {
 	struct bufferevent_openssl *bev_ssl = upcast(bev);
 
-	if (bev_ssl->underlying) {
-		bufferevent_del_generic_timeout_cbs_(bev);
-	} else {
-		event_del(&bev->ev_read);
-		event_del(&bev->ev_write);
-	}
-
 	if (bev_ssl->bev.options & BEV_OPT_CLOSE_ON_FREE) {
 		if (bev_ssl->underlying) {
 			if (BEV_UPCAST(bev_ssl->underlying)->refcnt < 2) {
@@ -1194,17 +1193,11 @@ be_openssl_destruct(struct bufferevent *bev)
 				    "bufferevent with too few references");
 			} else {
 				bufferevent_free(bev_ssl->underlying);
-				bev_ssl->underlying = NULL;
+				/* We still have a reference to it, since DOCUMENT. So we don't
+				 * drop this. */
+				// bev_ssl->underlying = NULL;
 			}
-		} else {
-			evutil_socket_t fd = -1;
-			BIO *bio = SSL_get_wbio(bev_ssl->ssl);
-			if (bio)
-				fd = BIO_get_fd(bio, NULL);
-			if (fd >= 0)
-				evutil_closesocket(fd);
 		}
-		SSL_free(bev_ssl->ssl);
 	} else {
 		if (bev_ssl->underlying) {
 			if (bev_ssl->underlying->errorcb == be_openssl_eventcb)
@@ -1216,6 +1209,24 @@ be_openssl_destruct(struct bufferevent *bev)
 	}
 }
 
+static void
+be_openssl_destruct(struct bufferevent *bev)
+{
+	struct bufferevent_openssl *bev_ssl = upcast(bev);
+
+	if (bev_ssl->bev.options & BEV_OPT_CLOSE_ON_FREE) {
+		if (! bev_ssl->underlying) {
+			evutil_socket_t fd = -1;
+			BIO *bio = SSL_get_wbio(bev_ssl->ssl);
+			if (bio)
+				fd = BIO_get_fd(bio, NULL);
+			if (fd >= 0)
+				evutil_closesocket(fd);
+		}
+		SSL_free(bev_ssl->ssl);
+	}
+}
+
 static int
 be_openssl_adj_timeouts(struct bufferevent *bev)
 {