On Thu, Jan 05, 2006 at 04:59:32PM +0000, Nick Kew wrote: > On Thursday 05 January 2006 16:13, Joe Orton wrote: > > On Thu, Jan 05, 2006 at 04:36:10PM +0100, Ruediger Pluem wrote: > > > I noticed that in ap_proxy_http_process_response of mod_proxy_http.c we > > > never call apr_brigade_destroy for the locally created brigade bb. > > > We only call apr_brigade_cleanup on this brigade. > > > Isn't this a memory leak? > > > > With the current allocation strategy for brigades, not really. > > > > Currently it's safer to call _cleanup rather than _destroy in most > > cases, since _destroy is equivalent to _cleanup but also unregisters the > > pool cleanup; so if a _destroy()ed brigade does get reused, it is at > > risk of leaking any contained buckets until the associated bucket > > allocator gets nuked. > > Hmmm. > > I was about to take issue with 'safer', since it's basically equivalent > (in that reusing after destroy() is logically a bug). > > I wonder if it would be worth flagging this, at least when compiled > with debug, by setting a flag when a brigade is destroyed, and > at least logging a message if it gets reused. If there are nasties > floating around current code (ours or third-party), it could help > catch them.
The filter API design (or really, the lack thereof) means this problem is already prevalent; the first step is to nail the "who owns the passed-in brigade" decision in stone then go from there. joe
