You are viewing a plain text version of this content. The canonical link for it is here.
Posted to apreq-dev@httpd.apache.org by Brian McQueen <mc...@gmail.com> on 2006/11/13 07:01:01 UTC
memory leaking on POST
I am having major problems with my httpd growing on POSTs. Is there
some finalization of cleanup or apreq_handle_release kind of a thing
that missed out on? I can't see any source for the leaks. Everything
is coming from req->pool, so any allocs should be transient. There is
no leak until I start parsing the POSTs. The httpds grow and grow as
if something is never getting released. The apreq handle is coming
from req->pool. I've even tried directly destroying the uploaded
bucket brigade when the work is all done, in the logging phase.
Do you have any tips on where I should look for my problem?
Brian McQueen
Re: memory leaking on POST
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Philip M. Gollucci" <pg...@p6m7g8.com> writes:
> Joe Schaefer wrote:
>> "Philip M. Gollucci" <pg...@p6m7g8.com> writes:
>>
>>> Joe Schaefer wrote:
>>>
>>>> Until someone teaches apreq_brigade_fwrite about spool buckets,
>>>> it probably shouldn't be used for upload brigades. Sorry about that.
>>> I was looking at this a while ago -- APR/U have this function.
>> Really? What's it called?
> include/apr_file_io.h:APR_DECLARE(apr_status_t)
>>>> apr_file_writev(apr_file_t *thefile
Sorry, that function doesn't help us here.
--
Joe Schaefer
Re: memory leaking on POST
Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
Joe Schaefer wrote:
> "Philip M. Gollucci" <pg...@p6m7g8.com> writes:
>
>> Joe Schaefer wrote:
>>
>>> Until someone teaches apreq_brigade_fwrite about spool buckets,
>>> it probably shouldn't be used for upload brigades. Sorry about that.
>> I was looking at this a while ago -- APR/U have this function.
>
> Really? What's it called?
include/apr_file_io.h:APR_DECLARE(apr_status_t) apr_file_writev(apr_file_t *thefile
--
------------------------------------------------------------------------
Philip M. Gollucci (pgollucci@p6m7g8.com) 323.219.4708
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F
When I call your name, Girl, it starts to flame
Burning in my heart, Tearing it all apart..
No matter how I try My love I cannot hide....
Re: memory leaking on POST
Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
Joe Schaefer wrote:
> "Brian McQueen" <mc...@gmail.com> writes:
>
>> I'll test it out
>
> Don't bother, I'm pretty sure apreq's behavior is unacceptable
> with that patch. We call apreq_brigade_fwrite way too many
> times internally to be able to afford to leak a brigade struct
> on each invocation.
>
> I'll try to commit a proper patch to trunk soonish. I'm glad
> you found a solution that seems to be working for you, but I'm
> also more than a little skeptical that apreq is working correctly
> if you are simply deleting the original buckets passed into
> apreq_brigade_fwrite.
>
This is important enough I'd hold 2.09 off for it.
--
------------------------------------------------------------------------
Philip M. Gollucci (pgollucci@p6m7g8.com) 323.219.4708
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F
When I call your name, Girl, it starts to flame
Burning in my heart, Tearing it all apart..
No matter how I try My love I cannot hide....
Re: memory leaking on POST
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Brian McQueen" <mc...@gmail.com> writes:
> I'll test it out
Don't bother, I'm pretty sure apreq's behavior is unacceptable
with that patch. We call apreq_brigade_fwrite way too many
times internally to be able to afford to leak a brigade struct
on each invocation.
I'll try to commit a proper patch to trunk soonish. I'm glad
you found a solution that seems to be working for you, but I'm
also more than a little skeptical that apreq is working correctly
if you are simply deleting the original buckets passed into
apreq_brigade_fwrite.
--
Joe Schaefer
Re: memory leaking on POST
Posted by Brian McQueen <mc...@gmail.com>.
I'll test it out, but in the mean time I took your brigade fwrite and
a added delete_bucket line and it seem to be much better. I don't
care about the data after it hits the disk, so I delete each bucket
after writing it to the disk. I wish I got this patch the other day
though! I worked many long hours to get that one line workaround I
just mentioned. I'll test out this patch. Thanks!
On 11/14/06, Joe Schaefer <jo...@sunstarsys.com> wrote:
> "Brian McQueen" <mc...@gmail.com> writes:
>
> > What bucket type is SPOOL? I don't see it as a real bucket type. I'd
> > like to try out the patch here, but I can't duplicate it because I
> > don't know what a spool bucket is.
>
> See if this patch to library/util.c solves your leak problem.
> It compiles, but it's largely untested:
>
> Index: library/util.c
> ===================================================================
> --- library/util.c (revision 474179)
> +++ library/util.c (working copy)
> @@ -777,11 +777,16 @@
> {
> struct iovec v[APREQ_DEFAULT_NELTS];
> apr_status_t s;
> - apr_bucket *e;
> + apr_bucket *e, *first;
> int n = 0;
> *wlen = 0;
> + apr_bucket_brigade *tmp = apr_brigade_create(bb->p, bb->bucket_alloc);
>
> - for (e = APR_BRIGADE_FIRST(bb); e != APR_BRIGADE_SENTINEL(bb);
> + s = apreq_brigade_copy(tmp, bb);
> + if (s != APR_SUCCESS)
> + return s;
> +
> + for (e = APR_BRIGADE_FIRST(tmp); e != APR_BRIGADE_SENTINEL(tmp);
> e = APR_BUCKET_NEXT(e))
> {
> apr_size_t len;
> @@ -789,6 +794,9 @@
> s = apreq_fwritev(f, v, &n, &len);
> if (s != APR_SUCCESS)
> return s;
> + while ((first = APR_BRIGADE_FIRST(tmp)) != e)
> + apr_bucket_delete(first);
> +
> *wlen += len;
> }
> s = apr_bucket_read(e, (const char **)&(v[n].iov_base),
> @@ -805,6 +813,8 @@
> if (s != APR_SUCCESS)
> return s;
> *wlen += len;
> + while ((first = APR_BRIGADE_FIRST(tmp)) != e)
> + apr_bucket_delete(first);
> }
> return APR_SUCCESS;
> }
>
>
> --
> Joe Schaefer
>
>
Re: memory leaking on POST
Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
Joe Schaefer wrote:
> "Brian McQueen" <mc...@gmail.com> writes:
>
>> What bucket type is SPOOL? I don't see it as a real bucket type. I'd
>> like to try out the patch here, but I can't duplicate it because I
>> don't know what a spool bucket is.
>
> See if this patch to library/util.c solves your leak problem.
> It compiles, but it's largely untested:
>
> Index: library/util.c
Joe, I say +1 to both trunk and branches/v2_09 --
It passes all the tests with worker and prefork mpms in a variety of
perl / httpd / mod_perl combinations on freebsd.
I've also had it live on a low volume ithread enabled mp2 production site.
I'm not exactly sure how we would specifically test its not leaking,
but maybe we could use/tweak module/t/c-modules/apreq_upload_test.
--
------------------------------------------------------------------------
Philip M. Gollucci (pgollucci@p6m7g8.com) 323.219.4708
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F
I never had a dream come true
'Til the day that I found you.
Even though I pretend that I've moved on
You'll always be my baby.
I never found the words to say
You're the one I think about each day
And I know no matter where life takes me to
A part of me will always be...
A part of me will always be with you.
Re: memory leaking on POST
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Brian McQueen" <mc...@gmail.com> writes:
> What bucket type is SPOOL? I don't see it as a real bucket type. I'd
> like to try out the patch here, but I can't duplicate it because I
> don't know what a spool bucket is.
See if this patch to library/util.c solves your leak problem.
It compiles, but it's largely untested:
Index: library/util.c
===================================================================
--- library/util.c (revision 474179)
+++ library/util.c (working copy)
@@ -777,11 +777,16 @@
{
struct iovec v[APREQ_DEFAULT_NELTS];
apr_status_t s;
- apr_bucket *e;
+ apr_bucket *e, *first;
int n = 0;
*wlen = 0;
+ apr_bucket_brigade *tmp = apr_brigade_create(bb->p, bb->bucket_alloc);
- for (e = APR_BRIGADE_FIRST(bb); e != APR_BRIGADE_SENTINEL(bb);
+ s = apreq_brigade_copy(tmp, bb);
+ if (s != APR_SUCCESS)
+ return s;
+
+ for (e = APR_BRIGADE_FIRST(tmp); e != APR_BRIGADE_SENTINEL(tmp);
e = APR_BUCKET_NEXT(e))
{
apr_size_t len;
@@ -789,6 +794,9 @@
s = apreq_fwritev(f, v, &n, &len);
if (s != APR_SUCCESS)
return s;
+ while ((first = APR_BRIGADE_FIRST(tmp)) != e)
+ apr_bucket_delete(first);
+
*wlen += len;
}
s = apr_bucket_read(e, (const char **)&(v[n].iov_base),
@@ -805,6 +813,8 @@
if (s != APR_SUCCESS)
return s;
*wlen += len;
+ while ((first = APR_BRIGADE_FIRST(tmp)) != e)
+ apr_bucket_delete(first);
}
return APR_SUCCESS;
}
--
Joe Schaefer
Re: memory leaking on POST
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Brian McQueen" <mc...@gmail.com> writes:
> What bucket type is SPOOL? I don't see it as a real bucket type. I'd
> like to try out the patch here, but I can't duplicate it because I
> don't know what a spool bucket is. Do you mean HEAP?
"Spool" buckets are essentially apreq-private file buckets; they
are implemented in library/util.c.
--
Joe Schaefer
Re: memory leaking on POST
Posted by Brian McQueen <mc...@gmail.com>.
What bucket type is SPOOL? I don't see it as a real bucket type. I'd
like to try out the patch here, but I can't duplicate it because I
don't know what a spool bucket is. Do you mean HEAP?
On 11/13/06, Joe Schaefer <jo...@sunstarsys.com> wrote:
> "Philip M. Gollucci" <pg...@p6m7g8.com> writes:
>
> > Joe Schaefer wrote:
> >
> >> Until someone teaches apreq_brigade_fwrite about spool buckets,
> >> it probably shouldn't be used for upload brigades. Sorry about that.
> > I was looking at this a while ago -- APR/U have this function.
>
> Really? What's it called?
>
> > We custom coded our own -- why ?
>
> Because I didn't know apr has something like it? In any case,
> the "mistake" here is that apreq_brigade_fwrite doesn't
> consume the brigade, but leaves it "intact". I should
> have made that function consume the brigade instead- my bad.
>
> The fix is easy enough- just use apr_file_read instead of
> apr_bucket_read when dealing with spool buckets. That'd fix
> the memory problem for this particular use-case. I wrote a patch
> last night, but I haven't had the opportunity to write any tests
> for it yet, and probably won't until the weekend comes around.
>
> --
> Joe Schaefer
>
>
Re: memory leaking on POST
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Philip M. Gollucci" <pg...@p6m7g8.com> writes:
> Joe Schaefer wrote:
>
>> Until someone teaches apreq_brigade_fwrite about spool buckets,
>> it probably shouldn't be used for upload brigades. Sorry about that.
> I was looking at this a while ago -- APR/U have this function.
Really? What's it called?
> We custom coded our own -- why ?
Because I didn't know apr has something like it? In any case,
the "mistake" here is that apreq_brigade_fwrite doesn't
consume the brigade, but leaves it "intact". I should
have made that function consume the brigade instead- my bad.
The fix is easy enough- just use apr_file_read instead of
apr_bucket_read when dealing with spool buckets. That'd fix
the memory problem for this particular use-case. I wrote a patch
last night, but I haven't had the opportunity to write any tests
for it yet, and probably won't until the weekend comes around.
--
Joe Schaefer
Re: memory leaking on POST
Posted by "Philip M. Gollucci" <pg...@p6m7g8.com>.
Joe Schaefer wrote:
> "Brian McQueen" <mc...@gmail.com> writes:
>
>> I do believe it has to do with my handling of the brigade, but I see
>> nothing suspicious. The data is parsed with apreq_body. On a
>> successful parse I get the pointer to the upload brigade from the
>> parsed parameter, and I pass that pointer to a function, which gets a
>> file handle and does apreq_brigade_fwrite. The file handle is then
>> closed. On errors I do nothing with the upload brigade. Its pretty
>> simple. Does that sound right? Any tips about safe handling of
>> parameters or pointers to brigades?
>
> Until someone teaches apreq_brigade_fwrite about spool buckets,
> it probably shouldn't be used for upload brigades. Sorry about that.
I was looking at this a while ago -- APR/U have this function.
We custom coded our own -- why ?
--
------------------------------------------------------------------------
Philip M. Gollucci (pgollucci@p6m7g8.com) 323.219.4708
Consultant / http://p6m7g8.net/Resume/resume.shtml
Senior Software Engineer - TicketMaster - http://ticketmaster.com
1024D/A79997FA F357 0FDD 2301 6296 690F 6A47 D55A 7172 A799 97F
When I call your name, Girl, it starts to flame
Burning in my heart, Tearing it all apart..
No matter how I try My love I cannot hide....
Re: memory leaking on POST
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Brian McQueen" <mc...@gmail.com> writes:
> I do believe it has to do with my handling of the brigade, but I see
> nothing suspicious. The data is parsed with apreq_body. On a
> successful parse I get the pointer to the upload brigade from the
> parsed parameter, and I pass that pointer to a function, which gets a
> file handle and does apreq_brigade_fwrite. The file handle is then
> closed. On errors I do nothing with the upload brigade. Its pretty
> simple. Does that sound right? Any tips about safe handling of
> parameters or pointers to brigades?
Until someone teaches apreq_brigade_fwrite about spool buckets,
it probably shouldn't be used for upload brigades. Sorry about that.
--
Joe Schaefer
Re: memory leaking on POST
Posted by Brian McQueen <mc...@gmail.com>.
I do believe it has to do with my handling of the brigade, but I see
nothing suspicious. The data is parsed with apreq_body. On a
successful parse I get the pointer to the upload brigade from the
parsed parameter, and I pass that pointer to a function, which gets a
file handle and does apreq_brigade_fwrite. The file handle is then
closed. On errors I do nothing with the upload brigade. Its pretty
simple. Does that sound right? Any tips about safe handling of
parameters or pointers to brigades?
On 11/12/06, Joe Schaefer <jo...@sunstarsys.com> wrote:
> "Brian McQueen" <mc...@gmail.com> writes:
>
> > I am having major problems with my httpd growing on POSTs. Is there
> > some finalization of cleanup or apreq_handle_release kind of a thing
> > that missed out on? I can't see any source for the leaks. Everything
> > is coming from req->pool, so any allocs should be transient. There is
> > no leak until I start parsing the POSTs. The httpds grow and grow as
> > if something is never getting released. The apreq handle is coming
> > from req->pool. I've even tried directly destroying the uploaded
> > bucket brigade when the work is all done, in the logging phase.
> >
> > Do you have any tips on where I should look for my problem?
>
> You shouldn't need to clean up anything, the pool cleanup should
> take care of everything apreq allocates. How are you handling
> the upload brigades? It's possible to leak buckets if you're
> not careful.
>
> --
> Joe Schaefer
>
>
Re: memory leaking on POST
Posted by "Edward L. Abrams" <ab...@bitscrap.com>.
A while back, I wrote to the list about this also. Joe and some others
sent me some example code that didn't leak buckets. At the time, I
thought I the clean code was still leaking a bit, however, months later,
with no processes having serious troubles, I now suspect that my own
individual case was just transient buffers in memory just not being
cleaned up at a moment in time I was watching. If you would like me to
re-post the example code that has been working for me (even though it
still does produce what looks like transient growth spurts), I'd be
happy to.
Eddie
abrams <at> tabblo <dot> com
Joe Schaefer wrote:
> "Brian McQueen" <mc...@gmail.com> writes:
>
>
>> I am having major problems with my httpd growing on POSTs. Is there
>> some finalization of cleanup or apreq_handle_release kind of a thing
>> that missed out on? I can't see any source for the leaks. Everything
>> is coming from req->pool, so any allocs should be transient. There is
>> no leak until I start parsing the POSTs. The httpds grow and grow as
>> if something is never getting released. The apreq handle is coming
>> from req->pool. I've even tried directly destroying the uploaded
>> bucket brigade when the work is all done, in the logging phase.
>>
>> Do you have any tips on where I should look for my problem?
>>
>
> You shouldn't need to clean up anything, the pool cleanup should
> take care of everything apreq allocates. How are you handling
> the upload brigades? It's possible to leak buckets if you're
> not careful.
>
>
Re: memory leaking on POST
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Brian McQueen" <mc...@gmail.com> writes:
> I am having major problems with my httpd growing on POSTs. Is there
> some finalization of cleanup or apreq_handle_release kind of a thing
> that missed out on? I can't see any source for the leaks. Everything
> is coming from req->pool, so any allocs should be transient. There is
> no leak until I start parsing the POSTs. The httpds grow and grow as
> if something is never getting released. The apreq handle is coming
> from req->pool. I've even tried directly destroying the uploaded
> bucket brigade when the work is all done, in the logging phase.
>
> Do you have any tips on where I should look for my problem?
You shouldn't need to clean up anything, the pool cleanup should
take care of everything apreq allocates. How are you handling
the upload brigades? It's possible to leak buckets if you're
not careful.
--
Joe Schaefer