+1
> Am 05.06.2018 um 14:07 schrieb Plüm, Rüdiger, Vodafone Group
> <[email protected]>:
>
>
>
>> -----Ursprüngliche Nachricht-----
>> Von: Stefan Eissing <[email protected]>
>> Gesendet: Dienstag, 5. Juni 2018 13:27
>> An: [email protected]
>> Cc: [email protected]
>> Betreff: Re: Brigade memory lifetime & leaks
>>
>>
>>
>>> Am 05.06.2018 um 10:46 schrieb Joe Orton <[email protected]>:
>>>
>>> In 2.4's http_request.c there are two places doing:
>>>
>>> bb = apr_brigade_create(c->pool, c->bucket_alloc);
>>>
>>> to handle sending a FLUSH between requests and an EOR bucket which
>> both
>>> can't be done off r->pool. Because the brigade structure itself is
>>> allocated out of c->pool this means we leak (~64bytes) per request
>>> forever or up to request/conn limits.
>>
>> Ok, not good. The first seems only to be triggered when request
>> data is still pending, but the second one always happens.
>>
>>> We can thread a temp pool through to those functions and fix this,
>>> though because these core internal functions are in the public API (ha
>>> ha) this is the usual compat PITA, and I haven't worked out if that's
>>> more complex for the the async requesta processing call chain.
>>> As a PoC for non-async MPMs:
>>> http://people.apache.org/~jorton/2.4-use-temp-pool.patch
>>>
>>> Another choice is to allocate the brigade structure using the bucket
>>> allocator and actually free it on _destroy(). Anybody around who can
>>> remember why we used a pool allocation for that structure from the
>>> beginning?
>>
>> How about having the apr_bucket_brigade struct on the stack as an
>> option?
>
> The usual approach in many locations is to store the created bucket brigade
> and reuse it. How about the following (untested):
>
> Index: modules/http/http_request.c
> ===================================================================
> --- modules/http/http_request.c (revision 1832516)
> +++ modules/http/http_request.c (working copy)
> @@ -345,6 +345,16 @@
> return rv;
> }
>
> +#define RETRIEVE_BRIGADE_FROM_POOL(bb, key, pool, allocator) do { \
> + apr_pool_userdata_get((void **)&bb, key, pool); \
> + if (bb == NULL) { \
> + bb = apr_brigade_create(pool, allocator); \
> + apr_pool_userdata_setn((const void *)bb, key, NULL, pool); \
> + } \
> + else { \
> + apr_brigade_cleanup(bb); \
> + } \
> +} while(0)
>
> AP_DECLARE(void) ap_process_request_after_handler(request_rec *r)
> {
> @@ -357,7 +367,8 @@
> * this bucket is destroyed, the request will be logged and
> * its pool will be freed
> */
> - bb = apr_brigade_create(c->pool, c->bucket_alloc);
> + RETRIEVE_BRIGADE_FROM_POOL(bb,
> "ap_process_request_after_handler_brigade",
> + c->pool, c->bucket_alloc);
> b = ap_bucket_eor_create(c->bucket_alloc, r);
> APR_BRIGADE_INSERT_HEAD(bb, b);
>
> @@ -477,7 +488,8 @@
> ap_process_async_request(r);
>
> if (!c->data_in_input_filters) {
> - bb = apr_brigade_create(c->pool, c->bucket_alloc);
> + RETRIEVE_BRIGADE_FROM_POOL(bb, "ap_process_request_brigade",
> + c->pool, c->bucket_alloc);
> b = apr_bucket_flush_create(c->bucket_alloc);
> APR_BRIGADE_INSERT_HEAD(bb, b);
> rv = ap_pass_brigade(c->output_filters, bb);
>
>
> Regards
>
> Rüdiger
