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 Joe Schaefer <jo...@sunstarsys.com> on 2005/06/08 22:50:53 UTC
Re: Massive leak in Apache2::Upload?
Markus Wichitill <ma...@gmx.de> writes:
> Joe Schaefer wrote:
>> 1) grab our svn trunk and apply the attached patch to it.
>> 2) make clean; make
>> 3) cd module; make test
>> 4) ls -l /tmp
>
> No files in /tmp.
WAG: see if this patch helps any
Index: library/util.c
===================================================================
--- library/util.c (revision 171027)
+++ library/util.c (working copy)
@@ -1171,14 +1171,14 @@
return s;
/* This cast, when out_len = -1, is intentional */
- if ((apr_uint64_t)out_len < heap_limit) {
+ if ((apr_size_t)out_len < heap_limit) {
s = apr_brigade_length(in, 0, &in_len);
if (s != APR_SUCCESS)
return s;
/* This cast, when in_len = -1, is intentional */
- if ((apr_uint64_t)in_len < heap_limit - (apr_uint64_t)out_len) {
+ if ((apr_size_t)in_len < heap_limit - (apr_size_t)out_len) {
APR_BRIGADE_CONCAT(out, in);
return APR_SUCCESS;
}
@@ -1229,7 +1229,7 @@
* temp_file bucket.
*/
- while ((apr_uint64_t)wlen > FILE_BUCKET_LIMIT - last_out->length) {
+ while ((apr_size_t)wlen > FILE_BUCKET_LIMIT - last_out->length) {
apr_bucket *e;
apr_bucket_copy(last_out, &e);
--
Joe Schaefer
Re: Massive leak in Apache2::Upload?
Posted by Ville Skyttä <vi...@iki.fi>.
On Thu, 2005-06-16 at 01:23 +0300, Ville Skyttä wrote:
> On Wed, 2005-06-15 at 10:45 -0400, Joe Schaefer wrote:
>
> > Let me ask you guys my original question in another way:
> > while monitoring the process, would you say that the process size
> > grows gradually during the actual network IO, or does it jump
> > suddenly right after the network IO completes?
>
> I haven't been able to test the latest patch nor can I test the above
> right now (but will try tomorrow). But if memory serves me right, the
> ballooning happened definitely after, not during the network I/O.
Memory did serve me right, and the answer to the WAG #2 patch is
unfortunately nay here too.
Re: Massive leak in Apache2::Upload?
Posted by Ville Skyttä <vi...@iki.fi>.
On Wed, 2005-06-15 at 10:45 -0400, Joe Schaefer wrote:
> Let me ask you guys my original question in another way:
> while monitoring the process, would you say that the process size
> grows gradually during the actual network IO, or does it jump
> suddenly right after the network IO completes?
I haven't been able to test the latest patch nor can I test the above
right now (but will try tomorrow). But if memory serves me right, the
ballooning happened definitely after, not during the network I/O.
Re: Massive leak in Apache2::Upload?
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Markus Wichitill <ma...@gmx.de> writes:
> FWIW, with or without the last patch, my one Perl
> interpreter thread seems to grow the size of the upload
> only twice, e.g with a file of 10MB I get:
>
> No upload: 14MB
> 1. upload: 24MB
> 2. upload: 24MB
> 3. upload: 34MB
> 4. upload: 34MB
> n. upload: 34MB
Interesting. I'm still having a little trouble reproducing it,
although I do see some wierd stuff happening in /tmp during the
upload. Let me ask you guys my original question in another way:
while monitoring the process, would you say that the process size
grows gradually during the actual network IO, or does it jump
suddenly right after the network IO completes?
--
Joe Schaefer
Re: Massive leak in Apache2::Upload?
Posted by Markus Wichitill <ma...@gmx.de>.
Joe Schaefer wrote:
>>Thanks. What's troubling me though is that while both you and Ville
>>are seeing the leak, you're not reporting consistent behavior with
>>the tempfile creation.
Well, if one of us screwed up, it would have to be me, so I repeated the
first patch test, and now I get tempfiles, too. I think I didn't actually
"make test" the last time, but just uploads in my app. To make up for lost
time, I get six files, though:
-rw------- 1 nobody nobody 500039 Jun 10 23:28 apreqIYDo8Q
-rw------- 1 nobody nobody 500024 Jun 10 23:28 apreqIdnCg3
-rw------- 1 nobody nobody 500039 Jun 10 23:28 apreqJQRtVu
-rw------- 1 nobody nobody 500039 Jun 10 23:28 apreqORx4CF
-rw------- 1 nobody nobody 500024 Jun 10 23:28 apreqR10Owy
-rw------- 1 nobody nobody 500039 Jun 10 23:28 apreqsuTtGu
> WAG #2: please test (yeah or nay) with this patch. This fixes at least
> one corner case bug, but I don't know if it plugs your leak as well.
Nay.
FWIW, with or without the last patch, my one Perl interpreter thread seems
to grow the size of the upload only twice, e.g with a file of 10MB I get:
No upload: 14MB
1. upload: 24MB
2. upload: 24MB
3. upload: 34MB
4. upload: 34MB
n. upload: 34MB
Re: Massive leak in Apache2::Upload?
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Schaefer <jo...@sunstarsys.com> writes:
> Markus Wichitill <ma...@gmx.de> writes:
[...]
>> Doesn't fix the original problem for me either.
>
> Thanks. What's troubling me though is that while both you and Ville
> are seeing the leak, you're not reporting consistent behavior with
> the tempfile creation. So although we know it's the prefetch code
> that's gobbling up memory (since calling discard_request_body fixes
> the leak), I really don't know where to look for it.
WAG #2: please test (yeah or nay) with this patch. This fixes at least
one corner case bug, but I don't know if it plugs your leak as well.
Index: library/util.c
===================================================================
--- library/util.c (revision 171027)
+++ library/util.c (working copy)
@@ -1091,7 +1091,7 @@
#define BUCKET_IS_SPOOL(e) ((e)->type == &spool_bucket_type)
-#define FILE_BUCKET_LIMIT ((apr_size_t)-1 - 1)
+#define FILE_BUCKET_LIMIT (1024 * 1024 * 1024) /* 1 GB for simplicity */
static
void spool_bucket_destroy(void *data)
@@ -1171,14 +1171,14 @@
return s;
/* This cast, when out_len = -1, is intentional */
- if ((apr_uint64_t)out_len < heap_limit) {
+ if ((apr_size_t)out_len < heap_limit) {
s = apr_brigade_length(in, 0, &in_len);
if (s != APR_SUCCESS)
return s;
/* This cast, when in_len = -1, is intentional */
- if ((apr_uint64_t)in_len < heap_limit - (apr_uint64_t)out_len) {
+ if ((apr_size_t)in_len < heap_limit - (apr_size_t)out_len) {
APR_BRIGADE_CONCAT(out, in);
return APR_SUCCESS;
}
@@ -1229,7 +1229,7 @@
* temp_file bucket.
*/
- while ((apr_uint64_t)wlen > FILE_BUCKET_LIMIT - last_out->length) {
+ while ((apr_size_t)wlen > FILE_BUCKET_LIMIT - last_out->length) {
apr_bucket *e;
apr_bucket_copy(last_out, &e);
@@ -1238,6 +1238,7 @@
wlen -= FILE_BUCKET_LIMIT - last_out->length;
last_out->length = FILE_BUCKET_LIMIT;
last_out->type = &apr_bucket_type_file;
+ e->type = &spool_bucket_type;
APR_BRIGADE_INSERT_TAIL(out, e);
last_out = e;
--
Joe Schaefer
Re: Massive leak in Apache2::Upload?
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Markus Wichitill <ma...@gmx.de> writes:
> Ville Skyttä wrote:
>>>WAG: see if this patch helps any
>> Nope, no difference at all :( I _did_ get the files in /tmp using your
>> earlier debugging patch; was this supposed to change that somehow
>> (didn't test with it now), or was it a WAG at fixing the original
>> reported problem?
>
> Doesn't fix the original problem for me either.
Thanks. What's troubling me though is that while both you and Ville
are seeing the leak, you're not reporting consistent behavior with
the tempfile creation. So although we know it's the prefetch
code that's gobbling up memory (since calling discard_request_body fixes
the leak), I really don't know where to look for it. I'd really like
to fix this sometime soon, so we can start the 2.06 candidates...
--
Joe Schaefer
Re: Massive leak in Apache2::Upload?
Posted by Markus Wichitill <ma...@gmx.de>.
Ville Skyttä wrote:
>>WAG: see if this patch helps any
>
> Nope, no difference at all :( I _did_ get the files in /tmp using your
> earlier debugging patch; was this supposed to change that somehow
> (didn't test with it now), or was it a WAG at fixing the original
> reported problem?
Doesn't fix the original problem for me either.
Re: Massive leak in Apache2::Upload?
Posted by Ville Skyttä <vi...@iki.fi>.
On Wed, 2005-06-08 at 16:50 -0400, Joe Schaefer wrote:
> Markus Wichitill <ma...@gmx.de> writes:
>
> > Joe Schaefer wrote:
> >> 1) grab our svn trunk and apply the attached patch to it.
> >> 2) make clean; make
> >> 3) cd module; make test
> >> 4) ls -l /tmp
> >
> > No files in /tmp.
>
> WAG: see if this patch helps any
Nope, no difference at all :( I _did_ get the files in /tmp using your
earlier debugging patch; was this supposed to change that somehow
(didn't test with it now), or was it a WAG at fixing the original
reported problem?