Improve robustness for refcounting

Document that we do intend to double-decref underlying bufferevents under
some circumstances.  Check to make sure that we don't decref past 0.

buffer.c

@@ -435,6 +435,8 @@ _evbuffer_decref_and_unlock(struct evbuffer *buffer)
 	struct evbuffer_chain *chain, *next;
 	ASSERT_EVBUFFER_LOCKED(buffer);
 
+	EVUTIL_ASSERT(buffer->refcnt > 0);
+
 	if (--buffer->refcnt > 0) {
 		EVBUFFER_UNLOCK(buffer);
 		return;

bufferevent-internal.h

@@ -280,11 +280,12 @@ void bufferevent_incref(struct bufferevent *bufev);
 /** Internal: Lock bufev and increase its reference count.
  * unlocking it otherwise. */
 void _bufferevent_incref_and_lock(struct bufferevent *bufev);
-/** Internal: Increment the reference count on bufev. */
-void bufferevent_decref(struct bufferevent *bufev);
+/** Internal: Decrement the reference count on bufev.  Returns 1 if it freed
+ * the bufferevent.*/
+int bufferevent_decref(struct bufferevent *bufev);
 /** Internal: Drop the reference count on bufev, freeing as necessary, and
- * unlocking it otherwise. */
-void _bufferevent_decref_and_unlock(struct bufferevent *bufev);
+ * unlocking it otherwise.  Returns 1 if it freed the bufferevent. */
+int _bufferevent_decref_and_unlock(struct bufferevent *bufev);
 
 /** Internal: If callbacks are deferred and we have a read callback, schedule
  * a readcb.  Otherwise just run the readcb. */

bufferevent.c

@@ -504,16 +504,36 @@ _bufferevent_incref_and_lock(struct bufferevent *bufev)
 	++bufev_private->refcnt;
 }
 
-void
+#if 0
+static void
+_bufferevent_transfer_lock_ownership(struct bufferevent *donor,
+    struct bufferevent *recipient)
+{
+	struct bufferevent_private *d = BEV_UPCAST(donor);
+	struct bufferevent_private *r = BEV_UPCAST(recipient);
+	if (d->lock != r->lock)
+		return;
+	if (r->own_lock)
+		return;
+	if (d->own_lock) {
+		d->own_lock = 0;
+		r->own_lock = 1;
+	}
+}
+#endif
+
+int
 _bufferevent_decref_and_unlock(struct bufferevent *bufev)
 {
 	struct bufferevent_private *bufev_private =
 	    EVUTIL_UPCAST(bufev, struct bufferevent_private, bev);
 	struct bufferevent *underlying;
 
+	EVUTIL_ASSERT(bufev_private->refcnt > 0);
+
 	if (--bufev_private->refcnt) {
 		BEV_UNLOCK(bufev);
-		return;
+		return 0;
 	}
 
 	underlying = bufferevent_get_underlying(bufev);
@@ -550,21 +570,27 @@ _bufferevent_decref_and_unlock(struct bufferevent *bufev)
 	/* Free the actual allocated memory. */
 	mm_free(bufev - bufev->be_ops->mem_offset);
 
-	/* release the reference to underlying now that we no longer need
-	 * the reference to it.  This is mainly in case our lock is shared
-	 * with underlying.
+	/* Release the reference to underlying now that we no longer need the
+	 * reference to it.  We wait this long mainly in case our lock is
+	 * shared with underlying.
+	 *
+	 * The 'destruct' function will also drop a reference to underlying
+	 * if BEV_OPT_CLOSE_ON_FREE is set.
+	 *
 	 * XXX Should we/can we just refcount evbuffer/bufferevent locks?
 	 * It would probably save us some headaches.
 	 */
 	if (underlying)
 		bufferevent_decref(underlying);
+
+	return 1;
 }
 
-void
+int
 bufferevent_decref(struct bufferevent *bufev)
 {
 	BEV_LOCK(bufev);
-	_bufferevent_decref_and_unlock(bufev);
+	return _bufferevent_decref_and_unlock(bufev);
 }
 
 void

bufferevent_filter.c

@@ -187,6 +187,7 @@ bufferevent_filter_new(struct bufferevent *underlying,
 	}
 
 	bufev_f->underlying = underlying;
+
 	bufev_f->process_in = input_filter;
 	bufev_f->process_out = output_filter;
 	bufev_f->free_context = free_context;
@@ -212,8 +213,19 @@ be_filter_destruct(struct bufferevent *bev)
 	if (bevf->free_context)
 		bevf->free_context(bevf->context);
 
-	if (bevf->bev.options & BEV_OPT_CLOSE_ON_FREE)
-		bufferevent_free(bevf->underlying);
+	if (bevf->bev.options & BEV_OPT_CLOSE_ON_FREE) {
+		/* Yes, there is also a decref in bufferevent_decref.
+		 * That decref corresponds to the incref when we set
+		 * underlying for the first time.  This decref is an
+		 * extra one to remove the last reference.
+		 */
+		if (BEV_UPCAST(bevf->underlying)->refcnt < 2) {
+			event_warnx("BEV_OPT_CLOSE_ON_FREE set on an "
+			    "bufferevent with too few references");
+		} else {
+			bufferevent_free(bevf->underlying);
+		}
+	}
 
 	_bufferevent_del_generic_timeout_cbs(bev);
 }

bufferevent_openssl.c

@@ -1030,8 +1030,13 @@ be_openssl_destruct(struct bufferevent *bev)
 
 	if (bev_ssl->bev.options & BEV_OPT_CLOSE_ON_FREE) {
 		if (bev_ssl->underlying) {
-			bufferevent_free(bev_ssl->underlying);
-			bev_ssl->underlying = NULL;
+			if (BEV_UPCAST(bev_ssl->underlying)->refcnt < 2) {
+				event_warnx("BEV_OPT_CLOSE_ON_FREE set on an "
+				    "bufferevent with too few references");
+			} else {
+				bufferevent_free(bev_ssl->underlying);
+				bev_ssl->underlying = NULL;
+			}
 		}
 		SSL_free(bev_ssl->ssl);
 	}