Changed use of refcounts to make sure referenced chains are freed in all cases.

diff --git a/buffer.c b/buffer.c
index 28d6adb240ce893366aee65434fe34d05b457070..85fc57b8857efc8323e128b194076eac2aba433b 100644 (file)
--- a/buffer.c
+++ b/buffer.c
@@ -150,7 +150,6 @@ static struct evbuffer_chain *evbuffer_expand_singlechain(struct evbuffer *buf,
 static int evbuffer_ptr_subtract(struct evbuffer *buf, struct evbuffer_ptr *pos,
     size_t howfar);
 static inline void evbuffer_chain_incref(struct evbuffer_chain *chain);
-static inline void evbuffer_chain_decref(struct evbuffer_chain *chain);
 
 static struct evbuffer_chain *
 evbuffer_chain_new(size_t size)
@@ -178,18 +177,24 @@ evbuffer_chain_new(size_t size)
         */
        chain->buffer = EVBUFFER_CHAIN_EXTRA(u_char, chain);
 
+       chain->refcnt = 1;
+
        return (chain);
 }
 
 static inline void
 evbuffer_chain_free(struct evbuffer_chain *chain)
 {
-       if (CHAIN_PINNED(chain)) {
-               chain->flags |= EVBUFFER_DANGLING;
+       EVUTIL_ASSERT(chain->refcnt > 0);
+       if (--chain->refcnt > 0) {
+               // chain is still referenced by other chains
                return;
        }
-       if (chain->refcnt > 0) {
-               // chain is still referenced by other chains
+       
+       if (CHAIN_PINNED(chain)) {
+               // will get freed once no longer dangling
+               chain->refcnt++;
+               chain->flags |= EVBUFFER_DANGLING;
                return;
        }
        
@@ -230,7 +235,7 @@ evbuffer_chain_free(struct evbuffer_chain *chain)
                EVUTIL_ASSERT(info->source != NULL);
                EVUTIL_ASSERT(info->parent != NULL);
                EVBUFFER_LOCK(info->source);
-               evbuffer_chain_decref(info->parent);
+               evbuffer_chain_free(info->parent);
                _evbuffer_decref_and_unlock(info->source);
        }
 
@@ -328,14 +333,6 @@ evbuffer_chain_incref(struct evbuffer_chain *chain)
     ++chain->refcnt;
 }
 
-static inline void
-evbuffer_chain_decref(struct evbuffer_chain *chain)
-{
-    EVUTIL_ASSERT(chain->refcnt > 0);
-    --chain->refcnt;
-    // chain will be freed when parent buffer is released
-}
-
 struct evbuffer *
 evbuffer_new(void)
 {

diff --git a/evbuffer-internal.h b/evbuffer-internal.h
index 1d67973f1819d6e8b9735b9016795c42817481a9..17de88b852b732fec726c2355bacb47c6dc486f1 100644 (file)
--- a/evbuffer-internal.h
+++ b/evbuffer-internal.h
@@ -186,7 +186,7 @@ struct evbuffer_chain {
        /** a chain that is a referenced copy of another chain */
 #define EVBUFFER_MULTICAST     0x0080
 
-       /** number of multicast references to this chain */
+       /** number of references to this chain */
        int refcnt;
 
        /** Usually points to the read-write memory belonging to this

diff --git a/test/regress_buffer.c b/test/regress_buffer.c
index 00240d92b0ea6ebb162473ea09103cf82afef250..441423a90c8fe85e65a8d13d6e72f8bbe1d94099 100644 (file)
--- a/test/regress_buffer.c
+++ b/test/regress_buffer.c
@@ -1590,6 +1590,42 @@ end:
                evbuffer_free(buf2);
 }
 
+static void
+test_evbuffer_multicast_drain(void *ptr)
+{
+       const char chunk1[] = "If you have found the answer to such a problem";
+       const char chunk2[] = "you ought to write it up for publication";
+                         /* -- Knuth's "Notes on the Exercises" from TAOCP */
+       size_t len1 = strlen(chunk1), len2=strlen(chunk2);
+
+       struct evbuffer *buf1 = NULL, *buf2 = NULL;
+
+       buf1 = evbuffer_new();
+       tt_assert(buf1);
+
+       evbuffer_add(buf1, chunk1, len1);
+       evbuffer_add(buf1, ", ", 2);
+       evbuffer_add(buf1, chunk2, len2);
+       tt_int_op(evbuffer_get_length(buf1), ==, len1+len2+2);
+
+       buf2 = evbuffer_new();
+       tt_assert(buf2);
+
+    tt_int_op(evbuffer_add_buffer_reference(buf2, buf1), ==, 0);
+       tt_int_op(evbuffer_get_length(buf2), ==, len1+len2+2);
+    tt_int_op(evbuffer_drain(buf1, evbuffer_get_length(buf1)), ==, 0);
+       tt_int_op(evbuffer_get_length(buf2), ==, len1+len2+2);
+    tt_int_op(evbuffer_drain(buf2, evbuffer_get_length(buf2)), ==, 0);
+       evbuffer_validate(buf1);
+       evbuffer_validate(buf2);
+
+end:
+       if (buf1)
+               evbuffer_free(buf1);
+       if (buf2)
+               evbuffer_free(buf2);
+}
+
 /* Some cases that we didn't get in test_evbuffer() above, for more coverage. */
 static void
 test_evbuffer_prepend(void *ptr)
@@ -1881,6 +1917,7 @@ struct testcase_t evbuffer_testcases[] = {
        { "callbacks", test_evbuffer_callbacks, 0, NULL, NULL },
        { "add_reference", test_evbuffer_add_reference, 0, NULL, NULL },
        { "multicast", test_evbuffer_multicast, 0, NULL, NULL },
+       { "multicast_drain", test_evbuffer_multicast_drain, 0, NULL, NULL },
        { "prepend", test_evbuffer_prepend, TT_FORK, NULL, NULL },
        { "peek", test_evbuffer_peek, 0, NULL, NULL },
        { "freeze_start", test_evbuffer_freeze, 0, &nil_setup, (void*)"start" },