You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2006/01/05 16:36:10 UTC
Possible memory leak in mod_proxy_http.c?
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?
Regards
Rüdiger
Re: Possible memory leak in mod_proxy_http.c?
Posted by Joe Orton <jo...@redhat.com>.
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
Re: Possible memory leak in mod_proxy_http.c?
Posted by Nick Kew <ni...@webthing.com>.
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.
--
Nick Kew
Re: Possible memory leak in mod_proxy_http.c?
Posted by Joe Orton <jo...@redhat.com>.
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.
joe