Author: rhuijben
Date: Mon Nov 9 14:05:48 2015
New Revision: 1713435
URL: http://svn.apache.org/viewvc?rev=1713435&view=rev
Log:
Resolve an ugly pool lifetime issue that might be triggered by the recent
aggregate early destruction change in r1712806.
While very theoretical in this case, this case will be far more likely when
we start sending bodies with http/2.
* buckets/aggregate_buckets.c
(read_aggregate): Limit cases of early destroy.
* test/test_buckets.c
(test_limit_buckets): Extend test.
Modified:
serf/trunk/buckets/aggregate_buckets.c
serf/trunk/test/test_buckets.c
Modified: serf/trunk/buckets/aggregate_buckets.c
URL:
http://svn.apache.org/viewvc/serf/trunk/buckets/aggregate_buckets.c?rev=1713435&r1=1713434&r2=1713435&view=diff
==============================================================================
--- serf/trunk/buckets/aggregate_buckets.c (original)
+++ serf/trunk/buckets/aggregate_buckets.c Mon Nov 9 14:05:48 2015
@@ -265,15 +265,18 @@ static apr_status_t read_aggregate(serf_
* we are asked to perform a read operation - thus ensuring the
* proper read lifetime.
*/
- if (cur_vecs_used > 0) {
+ if (*vecs_used > 0) {
next_list = ctx->list->next;
ctx->list->next = ctx->done;
ctx->done = ctx->list;
ctx->list = next_list;
}
else {
- /* This bucket didn't add a single byte.
- We can destroy it directly */
+ /* The first bucket didn't add a single byte: we can destroy
+ it now. We only do that if we are not already keeping other
+ buckets alive. In test_limit_buckets() shows a case where
+ an early destroy would not be safe if the bucket is not the
+ first one. */
next_list = ctx->list;
ctx->list = next_list->next;
serf_bucket_destroy(next_list->bucket);
Modified: serf/trunk/test/test_buckets.c
URL:
http://svn.apache.org/viewvc/serf/trunk/test/test_buckets.c?rev=1713435&r1=1713434&r2=1713435&view=diff
==============================================================================
--- serf/trunk/test/test_buckets.c (original)
+++ serf/trunk/test/test_buckets.c Mon Nov 9 14:05:48 2015
@@ -2108,6 +2108,33 @@ static void test_limit_buckets(CuTest *t
status = read_all(limit, buffer, sizeof(buffer), &len);
CuAssertIntEquals(tc, SERF_ERROR_TRUNCATED_HTTP_RESPONSE, status);
serf_bucket_destroy(limit);
+
+ /* And now a really bad case of the 'different way' */
+ raw = SERF_BUCKET_SIMPLE_STRING("ABCDE", alloc);
+ limit = serf_bucket_limit_create(
+ serf_bucket_barrier_create(raw, alloc),
+ 5, alloc);
+ agg = serf_bucket_aggregate_create(alloc);
+ serf_bucket_aggregate_prepend(agg, limit);
+ serf_bucket_aggregate_append(agg, raw);
+
+ {
+ struct iovec vecs[12];
+ int vecs_read;
+
+ /* This used to trigger a problem via the aggregate bucket,
+ as reading the last part destroyed the data pointed to by
+ iovecs of the first */
+
+ CuAssertIntEquals(tc, APR_EOF,
+ serf_bucket_read_iovec(agg, SERF_READ_ALL_AVAIL,
+ 12, vecs, &vecs_read));
+
+ serf__copy_iovec(buffer, &len, vecs, vecs_read);
+
+ CuAssertIntEquals(tc, 5, len);
+ }
+ serf_bucket_destroy(agg);
}
/* Basic test for unframe buckets. */
