On Tue, Jun 05, 2018 at 12:07:31PM +0000, Plüm, Rüdiger, Vodafone Group wrote:
> The usual approach in many locations is to store the created bucket brigade
> and reuse it. How about the following (untested):
Yes, that works too, thanks! Passes test suite and fixes the leak,
though I we need to change the _destroy() to be a _cleanup() for safety,
and add another, as in attached.
I think my long standing hatred for this particular function is because
every single C API ever created in history with a foo_create()
foo_destroy() pair has memory allocated by the creator and deallocated
by the destructor. But this one is special!
We should remove apr_brigade_destroy() in APR 2, IMO, or switch to
bucket allocated brigades then.
Index: modules/http/http_request.c
===================================================================
--- modules/http/http_request.c (revision 1832932)
+++ 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);
@@ -383,7 +394,7 @@
*/
rv = ap_check_pipeline(c, bb, DEFAULT_LIMIT_BLANK_LINES);
c->data_in_input_filters = (rv == APR_SUCCESS);
- apr_brigade_destroy(bb);
+ apr_brigade_cleanup(bb);
if (c->cs)
c->cs->state = (c->aborted) ? CONN_STATE_LINGER
@@ -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);
@@ -490,6 +502,7 @@
ap_log_cerror(APLOG_MARK, APLOG_INFO, rv, c, APLOGNO(01581)
"flushing data to the client");
}
+ apr_brigade_cleanup(bb);
}
if (ap_extended_status) {
ap_time_process_request(c->sbh, STOP_PREQUEST);
