Author: rhuijben
Date: Wed Nov 4 19:51:54 2015
New Revision: 1712621
URL: http://svn.apache.org/viewvc?rev=1712621&view=rev
Log:
In preparation for really fixing the root cause revert the ostream_tail
creation to use a normal aggregate stream which does destroy the buckets
added to it when done. This partially reverts r1698925.
[The fix was really already implemented years ago when the writing_started
flag was added]
This adds a lot more flexibility to the buckets added as they get a callback
when done (it even fixes a memoryleak in the http2 code where I didn't expect
this behavior).
Temporarily just introduce a barrier bucket around the request to allow
separating the behavior change over multiple commits.
* buckets/aggregate_buckets.c
(aggregate_context_t): Remove flag.
(cleanup_aggregate,
create_aggregate): Update usage.
(serf__bucket_stream_create): Remove function.
(serf_aggregate_destroy_and_data): Update usage.
* outgoing.c
(do_conn_setup): Update creation.
(write_to_connection): Create barrier.
* serf_bucket_types.h
(serf__bucket_stream_create): Remove private function.
* ssltunnel.c
(serf__ssltunnel_connect): Update creation.
Modified:
serf/trunk/buckets/aggregate_buckets.c
serf/trunk/outgoing.c
serf/trunk/serf_bucket_types.h
serf/trunk/ssltunnel.c
Modified: serf/trunk/buckets/aggregate_buckets.c
URL:
http://svn.apache.org/viewvc/serf/trunk/buckets/aggregate_buckets.c?rev=1712621&r1=1712620&r2=1712621&view=diff
==============================================================================
--- serf/trunk/buckets/aggregate_buckets.c (original)
+++ serf/trunk/buckets/aggregate_buckets.c Wed Nov 4 19:51:54 2015
@@ -36,9 +36,6 @@ typedef struct aggregate_context_t {
serf_bucket_aggregate_eof_t hold_open;
void *hold_open_baton;
- /* Does this bucket own its children? !0 if yes, 0 if not. */
- int bucket_owner;
-
serf_config_t *config;
} aggregate_context_t;
@@ -54,9 +51,7 @@ static void cleanup_aggregate(aggregate_
while (ctx->done != NULL) {
next_list = ctx->done->next;
- if (ctx->bucket_owner) {
- serf_bucket_destroy(ctx->done->bucket);
- }
+ serf_bucket_destroy(ctx->done->bucket);
serf_bucket_mem_free(allocator, ctx->done);
ctx->done = next_list;
@@ -83,7 +78,6 @@ static aggregate_context_t *create_aggre
ctx->hold_open = NULL;
ctx->hold_open_baton = NULL;
ctx->config = NULL;
- ctx->bucket_owner = 1;
return ctx;
}
@@ -98,21 +92,6 @@ serf_bucket_t *serf_bucket_aggregate_cre
return serf_bucket_create(&serf_bucket_type_aggregate, allocator, ctx);
}
-serf_bucket_t *serf__bucket_stream_create(
- serf_bucket_alloc_t *allocator,
- serf_bucket_aggregate_eof_t fn,
- void *baton)
-{
- serf_bucket_t *bucket = serf_bucket_aggregate_create(allocator);
- aggregate_context_t *ctx = bucket->data;
-
- serf_bucket_aggregate_hold_open(bucket, fn, baton);
-
- ctx->bucket_owner = 0;
-
- return bucket;
-}
-
static void serf_aggregate_destroy_and_data(serf_bucket_t *bucket)
{
@@ -120,9 +99,7 @@ static void serf_aggregate_destroy_and_d
bucket_list_t *next_ctx;
while (ctx->list) {
- if (ctx->bucket_owner) {
- serf_bucket_destroy(ctx->list->bucket);
- }
+ serf_bucket_destroy(ctx->list->bucket);
next_ctx = ctx->list->next;
serf_bucket_mem_free(bucket->allocator, ctx->list);
ctx->list = next_ctx;
Modified: serf/trunk/outgoing.c
URL:
http://svn.apache.org/viewvc/serf/trunk/outgoing.c?rev=1712621&r1=1712620&r2=1712621&view=diff
==============================================================================
--- serf/trunk/outgoing.c (original)
+++ serf/trunk/outgoing.c Wed Nov 4 19:51:54 2015
@@ -249,9 +249,9 @@ static apr_status_t do_conn_setup(serf_c
}
if (conn->ostream_tail == NULL) {
- conn->ostream_tail = serf__bucket_stream_create(conn->allocator,
- detect_eof,
- conn);
+ conn->ostream_tail = serf_bucket_aggregate_create(conn->allocator);
+
+ serf_bucket_aggregate_hold_open(conn->ostream_tail, detect_eof, conn);
}
ostream = conn->ostream_tail;
@@ -874,7 +874,10 @@ static apr_status_t write_to_connection(
if (!request->writing_started) {
request->writing_started = 1;
- serf_bucket_aggregate_append(ostreamt, request->req_bkt);
+ serf_bucket_aggregate_append(
+ ostreamt,
+ serf_bucket_barrier_create(request->req_bkt,
+ request->allocator));
}
}
Modified: serf/trunk/serf_bucket_types.h
URL:
http://svn.apache.org/viewvc/serf/trunk/serf_bucket_types.h?rev=1712621&r1=1712620&r2=1712621&view=diff
==============================================================================
--- serf/trunk/serf_bucket_types.h (original)
+++ serf/trunk/serf_bucket_types.h Wed Nov 4 19:51:54 2015
@@ -224,21 +224,6 @@ void serf_bucket_aggregate_cleanup(
serf_bucket_t *serf_bucket_aggregate_create(
serf_bucket_alloc_t *allocator);
-/* Creates a stream bucket.
- A stream bucket is like an aggregate bucket, but:
- - it doesn't destroy its child buckets on cleanup
- - one can always keep adding child buckets, the handler FN should return
- APR_EOF when no more buckets will be added.
-
- Note: keep this factory function internal for now. If it turns out this
- bucket type is useful outside serf, we should make it an actual separate
- type.
- */
-serf_bucket_t *serf__bucket_stream_create(
- serf_bucket_alloc_t *allocator,
- serf_bucket_aggregate_eof_t fn,
- void *baton);
-
/** Transform @a bucket in-place into an aggregate bucket.
*
* It is callers responsibility to free resources held by the original
Modified: serf/trunk/ssltunnel.c
URL:
http://svn.apache.org/viewvc/serf/trunk/ssltunnel.c?rev=1712621&r1=1712620&r2=1712621&view=diff
==============================================================================
--- serf/trunk/ssltunnel.c (original)
+++ serf/trunk/ssltunnel.c Wed Nov 4 19:51:54 2015
@@ -189,9 +189,8 @@ apr_status_t serf__ssltunnel_connect(ser
ctx->uri = apr_psprintf(ctx->pool, "%s:%d", conn->host_info.hostname,
conn->host_info.port);
- conn->ssltunnel_ostream = serf__bucket_stream_create(conn->allocator,
- detect_eof,
- conn);
+ conn->ssltunnel_ostream = serf_bucket_aggregate_create(conn->allocator);
+ serf_bucket_aggregate_hold_open(conn->ssltunnel_ostream, detect_eof, conn);
serf__ssltunnel_request_create(conn,
setup_request,
