Apache Mod-Python - memory leak in request.readline()

Author

memory leak in request.readline()

Alexis Marrero

2006-08-10, 1:14 pm

All,

We are trying to nail down a memory leak that happens only when
documents are POSTed to the server.

For testing we have a short script that does:

while True:
dictionary_of_parameters = {'field1': 'a'*100000}
post('url...', dictionary_of_parameters)

Then we run "top" on the server and watch the server memory grow
without bound. Why do we know that the problem is in request.readline
()? If I go to mod_python.util.FieldStorage.read_to_boundary() and
add the following statement:

def read_to_boundary(...):
return True
...

as the first executable line in the function the memory does not grow.

I have read the req_readline a 1000 time and I can't figure out where
the problem is.

My config:
Python 2.4.1
mod_python 3.2.10

Our request handler does nothing other than using util.FieldStorage
(req) and req.write('hello').

I have some suspicion that it has to do with:
.....
19 * requestobject.c
20 *
21 * $Id: requestobject.c 420297 2006-07-09 13:53:06Z nlehuen $
22 *
23 */
.....
846 /* Free rbuff if we're done with it */
847 if (self->rbuff_pos >= self->rbuff_len && self->rbuff !=
NULL)
848 {
849 free(self->rbuff);
850 self->rbuff = NULL;
851 }
852

Though, I can't confirm.

/amn

Jim Gallacher

2006-08-10, 7:13 pm

Alexis Marrero wrote:
> All,
>
> We are trying to nail down a memory leak that happens only when
> documents are POSTed to the server.
>
> For testing we have a short script that does:
>
> while True:
> dictionary_of_parameters = {'field1': 'a'*100000}
> post('url...', dictionary_of_parameters)
>
> Then we run "top" on the server and watch the server memory grow without
> bound. Why do we know that the problem is in request.readline()? If I
> go to mod_python.util.FieldStorage.read_to_boundary() and add the
> following statement:
>
> def read_to_boundary(...):
> return True
> ...
>
> as the first executable line in the function the memory does not grow.
>
> I have read the req_readline a 1000 time and I can't figure out where
> the problem is.
>
>
> My config:
> Python 2.4.1
> mod_python 3.2.10
>
> Our request handler does nothing other than using util.FieldStorage(req)
> and req.write('hello').
>
> I have some suspicion that it has to do with:
> ....
> 19 * requestobject.c
> 20 *
> 21 * $Id: requestobject.c 420297 2006-07-09 13:53:06Z nlehuen $
> 22 *
> 23 */
> ....
> 846 /* Free rbuff if we're done with it */
> 847 if (self->rbuff_pos >= self->rbuff_len && self->rbuff != NULL)
> 848 {
> 849 free(self->rbuff);
> 850 self->rbuff = NULL;
> 851 }
> 852
>
> Though, I can't confirm.
>
>
> /amn

When we were fixing MODPYTHON-172 I was a little suspicious of that bit
of code as

When we were fixing MODPYTHON-172 I was a little suspicious of that bit
of code as well, but I couldn't see exactly where it might be going
wrong, and in the end the leak hunt led elsewhere.

How fast are you leaking memory with your test script? Which Apache mpm
are you using?

Jim

Alexis Marrero

2006-08-10, 7:13 pm

I'm using Apache 2.0.54.

In my test is leaking exactly 8 bytes per request.

/amn

On Aug 10, 2006, at 2:49 PM, Jim Gallacher wrote:

> Alexis Marrero wrote:
>
> When we were fixing MODPYTHON-172 I was a little suspicious of that
> bit
> of code as
>
>
> When we were fixing MODPYTHON-172 I was a little suspicious of that
> bit
> of code as well, but I couldn't see exactly where it might be going
> wrong, and in the end the leak hunt led elsewhere.
>
> How fast are you leaking memory with your test script? Which Apache
> mpm
> are you using?
>
> Jim
>
>

Alexis Marrero

2006-08-10, 7:13 pm

Jim,

The size of memory leaked depends to the size of the POST form field.

In my test if the form field size named field1 is 1 it leaks around
8B. When len(field1) = 2**22 it leaks ~240B.

So it's hard to tell. This one is not an obvious one...

/amn

On Aug 10, 2006, at 4:09 PM, Alexis Marrero wrote:

> I'm using Apache 2.0.54.
>
> In my test is leaking exactly 8 bytes per request.
>
>
> /amn
>
> On Aug 10, 2006, at 2:49 PM, Jim Gallacher wrote:
>
>

Alexis Marrero

2006-08-11, 1:13 pm

Experimenting on this issue, I noticed that neither of the following
set of "ifs" are ever met:

786 /* Free old rbuff as the old contents have been copied
over and
787 we are about to allocate a new rbuff. Perhaps this
could be reused
788 somehow? */
789 if (self->rbuff_pos >= self->rbuff_len && self->rbuff !=
NULL)
790 {
791 free(self->rbuff);
792 self->rbuff = NULL;
793 }

--------

846 /* Free rbuff if we're done with it */
847 if (self->rbuff_pos >= self->rbuff_len && self->rbuff !=
NULL)
848 {
849 free(self->rbuff);
850 self->rbuff = NULL;
851 }

I noticed that by putting some statements to write to the output
stream. They never execute.

/amn

On Aug 10, 2006, at 1:43 PM, Alexis Marrero wrote:

> All,
>
> We are trying to nail down a memory leak that happens only when
> documents are POSTed to the server.
>
> For testing we have a short script that does:
>
> while True:
> dictionary_of_parameters = {'field1': 'a'*100000}
> post('url...', dictionary_of_parameters)
>
> Then we run "top" on the server and watch the server memory grow
> without bound. Why do we know that the problem is in
> request.readline()? If I go to
> mod_python.util.FieldStorage.read_to_boundary() and add the
> following statement:
>
> def read_to_boundary(...):
> return True
> ...
>
> as the first executable line in the function the memory does not grow.
>
> I have read the req_readline a 1000 time and I can't figure out
> where the problem is.
>
>
> My config:
> Python 2.4.1
> mod_python 3.2.10
>
> Our request handler does nothing other than using util.FieldStorage
> (req) and req.write('hello').
>
> I have some suspicion that it has to do with:
> ....
> 19 * requestobject.c
> 20 *
> 21 * $Id: requestobject.c 420297 2006-07-09 13:53:06Z nlehuen $
> 22 *
> 23 */
> ....
> 846 /* Free rbuff if we're done with it */
> 847 if (self->rbuff_pos >= self->rbuff_len && self->rbuff !
> = NULL)
> 848 {
> 849 free(self->rbuff);
> 850 self->rbuff = NULL;
> 851 }
> 852
>
> Though, I can't confirm.
>
>
> /amn
>

Alexis Marrero

2006-08-11, 1:13 pm

I noticed that lines 789-793 (in the original file, $Id: requestobject.c
420297) should be moved up to line 780,

765 if (self->rbuff_pos < self->rbuff_len) {
766
767 /* if yes, process that first */
768 while (self->rbuff_pos < self->rbuff_len) {
769
770 buffer[copied++] = self->rbuff[self->rbuff_pos];
771 if ((self->rbuff[self->rbuff_pos++] == '\n') ||
772 (copied == len)) {
773
774 /* our work is done */
775
776 /* resize if necessary */
777 if (copied < len)
778 if(_PyString_Resize(&result, copied))
779 return NULL;
780
781 return result;
782 }
783 }
784 }

So it should look like this:

if (self->rbuff_pos < self->rbuff_len) {

/* if yes, process that first */
while (self->rbuff_pos < self->rbuff_len) {

buffer[copied++] = self->rbuff[self->rbuff_pos];
if ((self->rbuff[self->rbuff_pos++] == '\n') ||
(copied == len)) {

/* our work is done */

/* resize if necessary */
if (copied < len)
if(_PyString_Resize(&result, copied))
return NULL;

if (self->rbuff_pos >= self->rbuff_len && self->rbuff !=
NULL) { // dispose of buffer since we no longer need it
free(self->rbuff);
self->rbuff = NULL;
}
return result;
}
}
}

The reason is that we are returning results, line 781 (original file),
as the last time that we will ever read from buffer without deallocating
the buffer memory.

Even with this fix, which I'm not 100% sure that is a fix, memory still
leaking but in a lower rate.

Any help will be greatly appreciated.

/amn
Alexis Marrero wrote:
> Experimenting on this issue, I noticed that neither of the following
> set of "ifs" are ever met:
>
>
> 786 /* Free old rbuff as the old contents have been copied
> over and
> 787 we are about to allocate a new rbuff. Perhaps this
> could be reused
> 788 somehow? */
> 789 if (self->rbuff_pos >= self->rbuff_len && self->rbuff !=
> NULL)
> 790 {
> 791 free(self->rbuff);
> 792 self->rbuff = NULL;
> 793 }
>
>
> --------
>
> 846 /* Free rbuff if we're done with it */
> 847 if (self->rbuff_pos >= self->rbuff_len && self->rbuff !=
> NULL)
> 848 {
> 849 free(self->rbuff);
> 850 self->rbuff = NULL;
> 851 }
>
> I noticed that by putting some statements to write to the output
> stream. They never execute.
>
> /amn
>
> On Aug 10, 2006, at 1:43 PM, Alexis Marrero wrote:
>
>

Jim Gallacher

2006-08-11, 1:13 pm

I'll have some time to investigate this over the next couple of days. I
ran my leaktest script for FieldStorage and readline, and FieldStorage
certainly still leaks, but I'm not so sure about readline itself.

baseline 1k requests 1.2%
readline 500k requests 1.6%
fieldstorage 498k requests 10.1%

The memory consumption figures are for a machine with 512MB ram.

I'm running my baseline test with 500k requests right now to see if the
1.6% figure for readline represents a real leak in that function, or if
it is just mod_python itself.

My memory leak test suite is probably at the point that other people
will find it useful. Once I've written a README explaining its use I'll
commit it to the repository so everybody to play. If you anyone wants to
give it a shot in the interim I can email it to you. Give me shout
offlist.

I haven't had a chance to look at the code you highlight below, or at
least not closely. The whole req_readline function looks like it will
require a good strong cup of coffee to fully comprehend. ;)

Jim

Alexis Marrero wrote:
> Experimenting on this issue, I noticed that neither of the following set
> of "ifs" are ever met:
>
>
> 786 /* Free old rbuff as the old contents have been copied over and
> 787 we are about to allocate a new rbuff. Perhaps this could
> be reused
> 788 somehow? */
> 789 if (self->rbuff_pos >= self->rbuff_len && self->rbuff != NULL)
> 790 {
> 791 free(self->rbuff);
> 792 self->rbuff = NULL;
> 793 }
>
>
> --------
>
> 846 /* Free rbuff if we're done with it */
> 847 if (self->rbuff_pos >= self->rbuff_len && self->rbuff != NULL)
> 848 {
> 849 free(self->rbuff);
> 850 self->rbuff = NULL;
> 851 }
>
> I noticed that by putting some statements to write to the output
> stream. They never execute.
>
> /amn
>
> On Aug 10, 2006, at 1:43 PM, Alexis Marrero wrote:
>
>
>

Jim Gallacher

2006-08-11, 1:13 pm

I ran my baseline test with 500k requests, and got the following:
(Note that all the figures will have an error of +/- 0.1)

baseline 500k requests 1.7%

So it would seem that there is not a specific problem in readline, or my
test case is messed up. FYI here are my 2 handlers:

def baseline_handler(req):
req.content_type = 'text/plain'
req.write('ok baseline:')
return apache.OK

def readline_handler(req):
# the body of the request consists of
# '\n'.join([ 'a'*10 for i in xrange(0,10) ])
req.content_type = 'text/plain'
count = 0
while(1):
line = req.readline()
if not line:
break
count += 1

req.write('ok readline: %d lines read' % count)
return apache.OK

Jim

Jim Gallacher wrote:
> I'll have some time to investigate this over the next couple of days. I
> ran my leaktest script for FieldStorage and readline, and FieldStorage
> certainly still leaks, but I'm not so sure about readline itself.
>
> baseline 1k requests 1.2%
> readline 500k requests 1.6%
> fieldstorage 498k requests 10.1%
>
> The memory consumption figures are for a machine with 512MB ram.
>
> I'm running my baseline test with 500k requests right now to see if the
> 1.6% figure for readline represents a real leak in that function, or if
> it is just mod_python itself.
>
> My memory leak test suite is probably at the point that other people
> will find it useful. Once I've written a README explaining its use I'll
> commit it to the repository so everybody to play. If you anyone wants to
> give it a shot in the interim I can email it to you. Give me shout
> offlist.
>
> I haven't had a chance to look at the code you highlight below, or at
> least not closely. The whole req_readline function looks like it will
> require a good strong cup of coffee to fully comprehend. ;)
>
> Jim
>
> Alexis Marrero wrote:
>
>

Alexis Marrero

2006-08-11, 7:12 pm

Jim,

I found the culprit!!!

There are two unrelated memory leaks.

The first one is in req_readline().

This code:

/* is there anything left in the rbuff from previous reads? */
if (self->rbuff_pos < self->rbuff_len) {

/* if yes, process that first */
while (self->rbuff_pos < self->rbuff_len) {
buffer[copied++] = self->rbuff[self->rbuff_pos];
if ((self->rbuff[self->rbuff_pos++] == '\n') ||
(copied == len)) {

/* our work is done */

/* resize if necessary */
if (copied < len)
if(_PyString_Resize(&result, copied))
return NULL;
return result;
}
}
}

Should look like this:
/* is there anything left in the rbuff from previous reads? */
if (self->rbuff_pos < self->rbuff_len) {

/* if yes, process that first */
while (self->rbuff_pos < self->rbuff_len) {
buffer[copied++] = self->rbuff[self->rbuff_pos];
if ((self->rbuff[self->rbuff_pos++] == '\n') ||
(copied == len)) {

/* our work is done */

/* resize if necessary */
if (copied < len)
if(_PyString_Resize(&result, copied))
return NULL;
if (self->rbuff_pos >= self->rbuff_len && self->rbuff !=
NULL)
{
free(self->rbuff);
self->rbuff = NULL;
}
return result;
}
}
}

That solves one. Like I mentioned in one of the emails to the mailing
list, the buffer was not been freed in the last readline().

The second one, for which I don't have a fix yet is apache.make_table()
in mod_python/util.py line 152. If I comment lines 152, 225, 227 you
will see that memory doesn't grow. I will keep investigating...

Until the next email.

/amn
Jim Gallacher wrote:
> I ran my baseline test with 500k requests, and got the following:
> (Note that all the figures will have an error of +/- 0.1)
>
> baseline 500k requests 1.7%
>
>
> So it would seem that there is not a specific problem in readline, or my
> test case is messed up. FYI here are my 2 handlers:
>
> def baseline_handler(req):
> req.content_type = 'text/plain'
> req.write('ok baseline:')
> return apache.OK
>
>
> def readline_handler(req):
> # the body of the request consists of
> # '\n'.join([ 'a'*10 for i in xrange(0,10) ])
> req.content_type = 'text/plain'
> count = 0
> while(1):
> line = req.readline()
> if not line:
> break
> count += 1
>
> req.write('ok readline: %d lines read' % count)
> return apache.OK
>
> Jim
>
>
> Jim Gallacher wrote:
>
>
>

Alexis Marrero

2006-08-11, 7:12 pm

Where is the source code for _apache.make_table() ?

Alexis Marrero wrote:
> Jim,
>
> I found the culprit!!!
>
> There are two unrelated memory leaks.
>
> The first one is in req_readline().
>
> This code:
>
> /* is there anything left in the rbuff from previous reads? */
> if (self->rbuff_pos < self->rbuff_len) {
> /* if yes, process that first */
> while (self->rbuff_pos < self->rbuff_len) {
> buffer[copied++] = self->rbuff[self->rbuff_pos];
> if ((self->rbuff[self->rbuff_pos++] == '\n') ||
> (copied == len)) {
>
> /* our work is done */
>
> /* resize if necessary */
> if (copied < len)
> if(_PyString_Resize(&result, copied))
> return NULL;
> return result;
> }
> }
> }
>
> Should look like this:
> /* is there anything left in the rbuff from previous reads? */
> if (self->rbuff_pos < self->rbuff_len) {
> /* if yes, process that first */
> while (self->rbuff_pos < self->rbuff_len) {
> buffer[copied++] = self->rbuff[self->rbuff_pos];
> if ((self->rbuff[self->rbuff_pos++] == '\n') ||
> (copied == len)) {
>
> /* our work is done */
>
> /* resize if necessary */
> if (copied < len)
> if(_PyString_Resize(&result, copied))
> return NULL;
> if (self->rbuff_pos >= self->rbuff_len && self->rbuff
> != NULL)
> {
> free(self->rbuff);
> self->rbuff = NULL;
> }
> return result;
> }
> }
> }
>
> That solves one. Like I mentioned in one of the emails to the mailing
> list, the buffer was not been freed in the last readline().
>
> The second one, for which I don't have a fix yet is
> apache.make_table() in mod_python/util.py line 152. If I comment lines
> 152, 225, 227 you will see that memory doesn't grow. I will keep
> investigating...
>
> Until the next email.
>
> /amn
> Jim Gallacher wrote:
>

Jim Gallacher

2006-08-11, 7:12 pm

I've created a JIRA issue for the readline leaks. The one I detail is a
corner case related to what you found, but I don't think the fix below
will help. Take a look at 182 and let me know what you think.
http://issues.apache.org/jira/browse/MODPYTHON-182

I think we should be checking requestobject self->rbuff during the
request cleanup and make sure it really is NULL, just as a safety check.

Alexis Marrero wrote:
> Jim,
>
> I found the culprit!!!
>
> There are two unrelated memory leaks.
>
> The first one is in req_readline().
>
> This code:
>
> /* is there anything left in the rbuff from previous reads? */
> if (self->rbuff_pos < self->rbuff_len) {
> /* if yes, process that first */
> while (self->rbuff_pos < self->rbuff_len) {
> buffer[copied++] = self->rbuff[self->rbuff_pos];
> if ((self->rbuff[self->rbuff_pos++] == '\n') ||
> (copied == len)) {
>
> /* our work is done */
>
> /* resize if necessary */
> if (copied < len)
> if(_PyString_Resize(&result, copied))
> return NULL;
> return result;
> }
> }
> }
>
> Should look like this:
> /* is there anything left in the rbuff from previous reads? */
> if (self->rbuff_pos < self->rbuff_len) {
> /* if yes, process that first */
> while (self->rbuff_pos < self->rbuff_len) {
> buffer[copied++] = self->rbuff[self->rbuff_pos];
> if ((self->rbuff[self->rbuff_pos++] == '\n') ||
> (copied == len)) {
>
> /* our work is done */
>
> /* resize if necessary */
> if (copied < len)
> if(_PyString_Resize(&result, copied))
> return NULL;
> if (self->rbuff_pos >= self->rbuff_len && self->rbuff !=
> NULL)
> {
> free(self->rbuff);
> self->rbuff = NULL;
> }
> return result;
> }
> }
> }
>
> That solves one. Like I mentioned in one of the emails to the mailing
> list, the buffer was not been freed in the last readline().

But not completely - see MODPYTHON-182.

> The second one, for which I don't have a fix yet is apache.make_table()
> in mod_python/util.py line 152. If I comment lines 152, 225, 227 you
> will see that memory doesn't grow. I will keep investigating...

As will I ...

Jim

> Until the next email.
>
> /amn
> Jim Gallacher wrote:
>
>

Alexis Marrero

2006-08-11, 7:12 pm

I forgot to mentioned that I changed request_tp_dealloc to:
static void request_tp_dealloc(requestobject *self)
{
// de-register the object from the GC
// before its deallocation, to prevent the
// GC to run on a partially de-allocated object
if (self->rbuff != NULL) {
free(self->rbuff);
}
PyObject_GC_UnTrack(self);

request_tp_clear(self);

PyObject_GC_Del(self);
}

I don't know if that function will be the right place to
free(self->rbuff) but it for the mean time there is no leak in my test.

Jim Gallacher wrote:
> I've created a JIRA issue for the readline leaks. The one I detail is a
> corner case related to what you found, but I don't think the fix below
> will help. Take a look at 182 and let me know what you think.
> http://issues.apache.org/jira/browse/MODPYTHON-182
>
> I think we should be checking requestobject self->rbuff during the
> request cleanup and make sure it really is NULL, just as a safety check.
>
> Alexis Marrero wrote:
>
>
> But not completely - see MODPYTHON-182.
>
>
>
> As will I ...
>
> Jim
>
>
>
>

Jim Gallacher

2006-08-11, 7:12 pm

Alexis Marrero wrote:
> I forgot to mentioned that I changed request_tp_dealloc to:
> static void request_tp_dealloc(requestobject *self)
> { // de-register the object from the GC
> // before its deallocation, to prevent the
> // GC to run on a partially de-allocated object
> if (self->rbuff != NULL) {
> free(self->rbuff);
> }
> PyObject_GC_UnTrack(self);
> request_tp_clear(self);
>
> PyObject_GC_Del(self);
> }
>
> I don't know if that function will be the right place to
> free(self->rbuff) but it for the mean time there is no leak in my test.

Somehow that doesn't feel like the right spot, but I'm not sure either.

Jim

Jim Gallacher

2006-08-11, 7:12 pm

Alexis Marrero wrote:
> Where is the source code for _apache.make_table() ?

It's a rather convoluted path, but src/tableobject.c is where you end up.

I've added a test to my leak test suite for apache.table().
(apache.make_table() was deprecated at some point). It does indeed leak
like crazy.

Jim

> Alexis Marrero wrote:
>
>

Graham Dumpleton

2006-08-13, 1:12 am

Where are we at with confirming these memory leaks and committing some
changes to fix them?

Only ask as I have a big patch related to MODPYTHON-63 that am wanting
to commit back into the repository, but don't want to be doing it if
it is going
to make your search harder by introducing some new memory leaks. :-)

BTW, what platform does you leak tester require to run? On Mac OS X I am
always seeing slow memory leaks over time even with most basic handler.
This often makes it hard to know if changes I make are introducing new
memory leaks or not. :-(

Graham

On 12/08/2006, at 7:55 AM, Jim Gallacher wrote:
[vbcol=seagreen]
> Alexis Marrero wrote:
>
> It's a rather convoluted path, but src/tableobject.c is where you
> end up.
>
> I've added a test to my leak test suite for apache.table().
> (apache.make_table() was deprecated at some point). It does indeed
> leak
> like crazy.
>
> Jim
>

Jim Gallacher

2006-08-13, 1:12 pm

Graham Dumpleton wrote:
> Where are we at with confirming these memory leaks and committing some
> changes to fix them?

Close.

I spent quite a few hours reading through req_readline yesterday, trying
to trace through all the possibilities. There are some... ah...
interesting things in there that look a little dodgey. The suspicious
bits are likely just corner case that may never happen, but could
corrupt memory or the readline output. I'll be soliciting feedback from
the list today.

If we are going to mess with the code in readline (and obviously we
are), I think we may as well do a quick optimization at the same time.
Currently a new buffer is malloc'd for each readline(len > 0). There is
a comment in the code wondering about reusing this buffer and I think I
see a fairly easy way to do it. This could be a nice little optimization
for people like Mike L. who are uploading large iso files. For a CD-ROM
sized file you would save approx 10000 malloc / free calls by reusing
the buffer - that seems worthwhile to me.

> Only ask as I have a big patch related to MODPYTHON-63 that am wanting
> to commit back into the repository, but don't want to be doing it if it
> is going
> to make your search harder by introducing some new memory leaks. :-)

As long as you don't touch req_readline it shouldn't matter. I'm looking
at the memory difference between a simple handler (baseline) and the
specific test anyway, so if you introduce a leak elsewhere it will be
accounted for.

> BTW, what platform does you leak tester require to run?

Top that supports the following options:
-b > batch mode
-u > user (assumes www-data)
-n > iterations
-d > delay
-p > pid

It also assumes that the %mem is in column 10 of the top output, but you
can obviously tweak the code if your's is different.

There is also the ability to dump the memory map with pmap, but this can
generate a hugh amount of data and is slow. Also I'm not entirely sure
what to do with output, so it's not on by default. If you don't have
pmap it's not a problem. (This memory dump also requires "ps -u
www-data", to get the pid to dump, but I would expect that output will
be the same on any *nix-like system).

The biggest limitation is the assumptions made about your Apache setup.
I'm running this on a test machine using the system's apache config.
There is only one apache instance running on the machine as user
www-data, which I'm using to filter the output from top. I'm also
starting and stopping apache after each test using:

sudo /etc/init.d/apache2 restart

There is also an assumption that the leak handlers are at
http://localhost/mod_python/leaktests

which on my system is a symlink in my document root to the tests.

ln -s $HOME/mod_python/debug/leaktest/handlers
/var/www/mod_python/leaktests

This test harness started out as a quick and dirty hack. Ultimately it
should have a configuration similar to our unit tests so it is
independent of the main apache configuration.

I wanted to tweak the test harness a little more before making it
available, but maybe I'll commit it to my sandbox as is. It is usable
enough, but just a little to specific to my test setup.

> On Mac OS X I am
> always seeing slow memory leaks over time even with most basic handler.
> This often makes it hard to know if changes I make are introducing new
> memory leaks or not. :-(

I'm convinced I'm seeing a small leak in the most basic handler as well,
but the leaks we've found and fixed so far have been serious enough to
overwhelm it so we really haven't noticed.

Jim

Graham Dumpleton

2006-08-14, 7:15 am

On 14/08/2006, at 1:16 AM, Jim Gallacher wrote:
>
> As long as you don't touch req_readline it shouldn't matter. I'm
> looking
> at the memory difference between a simple handler (baseline) and the
> specific test anyway, so if you introduce a leak elsewhere it will be
> accounted for.

Okay, see how you go with what I just checked in. I did though forget to
cross reference to changes in commit message. Actual changes are:

http://svn.apache.org/viewvc?view=rev&revision=431327

If it leaks badly let me know and I'll review my changes again.

Graham

Alexis Marrero

2006-08-14, 1:12 pm

Graham,

In http://svn.apache.org/viewvc/httpd/...thon/trunk/src/
requestobject.c?view=markup&pathrev=431327, in req_readlines():

/* is there anything left in the rbuff from previous reads? */
if (self->rbuff_pos < self->rbuff_len) {

/* if yes, process that first */
while (self->rbuff_pos < self->rbuff_len) {

buffer[copied++] = self->rbuff[self->rbuff_pos];
if ((self->rbuff[self->rbuff_pos++] == '\n') ||
(copied == len)) {

/* our work is done */

/* resize if necessary */
if (copied < len)
if(_PyString_Resize(&result, copied))
return NULL;

return result; // RETURNING results
without free(buffer)
}
}
}

/* Free old rbuff as the old contents have been copied over and
we are about to allocate a new rbuff. Perhaps this could be
reused
somehow? */
if (self->rbuff_pos >= self->rbuff_len && self->rbuff != NULL)
{
free(self->rbuff);
self->rbuff = NULL;
}

There is one of the memory leaks.

Putting the if statement "if (self->rbuff_pos >= self->rbuff_len &&
self->rbuff != NULL)" just before the "return result;" fix that one.

/* is there anything left in the rbuff from previous reads? */
if (self->rbuff_pos < self->rbuff_len) {

/* if yes, process that first */
while (self->rbuff_pos < self->rbuff_len) {

buffer[copied++] = self->rbuff[self->rbuff_pos];
if ((self->rbuff[self->rbuff_pos++] == '\n') ||
(copied == len)) {

/* our work is done */

/* resize if necessary */
if (copied < len)
if(_PyString_Resize(&result, copied))
return NULL;

if (self->rbuff_pos >= self->rbuff_len && self-
>rbuff != NULL) {
free(self->rbuff);
self->rbuff = NULL;
}
return result;
}
}
}

We also need to free the buffer in the "tp_dealloc" or some other
function that is called when disposing the object.

/amn

On Aug 14, 2006, at 8:12 AM, Graham Dumpleton wrote:

>
> On 14/08/2006, at 1:16 AM, Jim Gallacher wrote:
>
> Okay, see how you go with what I just checked in. I did though
> forget to
> cross reference to changes in commit message. Actual changes are:
>
> http://svn.apache.org/viewvc?view=rev&revision=431327
>
> If it leaks badly let me know and I'll review my changes again.
>
> Graham