You are viewing a plain text version of this content. The canonical link for it is here.
Posted to mod_python-dev@quetz.apache.org by Jim Gallacher <jp...@jgassociates.ca> on 2006/07/10 14:17:57 UTC
memory leaks (was Re: [jira] Commented: (MODPYTHON-172) Memory leak
with util.fieldstorage using mod_python 3.2.8 on apache 2.0.55)
I've run some tests to evaluate the memory leaks. The tests are brute
force - make requests and watch the memory changes with top -b.
These tests were run using Ubuntu 6.06, Apache 2.0.55 (mpm-worker with
StartServers = 2) and Python 2.4.3.
The baseline test is just a simple request to determine the initial
memory consumption and in theory should not leak. The memory footprint
is checked after each 1000 requests, and if greater than the memory
maximum the test is terminated and marked as FAIL. Tests with PASS
status may still be leaking, just not in a dramatic, fatal manner.
First up - our leaky 3.2.9, and man does it leak! The readlines request
has a body of 1M bytes, and it fails in a hurry.
============================================================
Mod_python Memory Leak Test
mod_python version: 3.2.9
requests attempted: 50000
memory maximum for failure condition: 10.0%
============================================================
Test Name Status Requests %Mem
------------------------------------------------------------
baseline PASS 50000 1.1
cfgtree_walk FAIL 5000 11.5
parse_qs PASS 50000 2.2
parse_qsl FAIL 15000 10.3
readlines FAIL 1000 14.3
The following is the lastest from branches/3.2.x (includes backported
MODPYTHON-172 changes.)
============================================================
Mod_python Memory Leak Test
mod_python version: 3.2.x build 20060709
requests attempted: 50000
memory maximum for failure condition: 10.0%
============================================================
Test Name Status Requests %Mem
------------------------------------------------------------
baseline PASS 50000 1.1
cfgtree_walk PASS 50000 2.0
parse_qs PASS 50000 2.2
parse_qsl PASS 50000 2.1
readlines PASS 50000 1.3
Everything is looking much better, with readlines looking very stable.
Cfgtree_walk and parse_qsl are also looking much better but there may
still be a problem. To confirm this I ran the tests against 3.2.x build
20060709 again, but with 100,000 requests.
============================================================
Mod_python Memory Leak Test
mod_python version: 3.2.x build 20060709
requests attempted: 100000
memory maximum for failure condition: 10.0%
============================================================
Test Name Status Requests %Mem
------------------------------------------------------------
baseline PASS 100000 1.2
cfgtree_walk PASS 100000 2.9
parse_qs PASS 100000 3.0
parse_qsl PASS 100000 3.1
readlines PASS 100000 1.3
I'm still suspicious of cfgtree_walk, parse_qs and parse_qsl as their
memory consumption is creeping up. I've taken a quick look at the code
but I don't see any obvious problem. I'll re-run the tests tonight, but
with a much larger number of requests.
The next step is to look at other methods we should include for testing.
I would be nice to get 100% coverage, but I'm not sure that is really
practical. We should include the most commonly called methods as a minimum.
Jim
Jim Gallacher wrote:
> I'm running my leaktest suite against this commit now. Results to follow.
>
> Jim
>
> Nicolas Lehuen (JIRA) wrote:
>> [ http://issues.apache.org/jira/browse/MODPYTHON-172?page=comments#action_12419906 ]
>>
>> Nicolas Lehuen commented on MODPYTHON-172:
>> ------------------------------------------
>>
>> I've just backported the fix into the 3.2 branch.
>>
>>> Memory leak with util.fieldstorage using mod_python 3.2.8 on apache 2.0.55
>>> --------------------------------------------------------------------------
>>>
>>> Key: MODPYTHON-172
>>> URL: http://issues.apache.org/jira/browse/MODPYTHON-172
>>> Project: mod_python
>>> Type: Bug
>>> Components: core
>>> Versions: 3.2.8
>>> Environment: Win32 XP SP1 / SP2
>>> Apache 2.0.55 installed from binary (.MSI)
>>> Python 2.4.2 or 2.4.3 installed from binary from www.python.org
>>> Reporter: Laurent Blanquet
>>> Fix For: 3.3
>>> I encounter memory leaks [~ 16 K per request) using the configuration described below.
>>> =============================
>>> Python configuration from Httpd.conf:
>>> =============================
>>> Alias /python/ "d:/python24/B2B/"
>>> <Directory "d:/python24/B2B">
>>> AddHandler mod_python .py
>>> PythonHandler pyHandlerHTTP
>>> PythonDebug On
>>> </Directory>
>>> =============================
>>> Test handler - pyHandlerHTTP.py :
>>> =============================
>>> import mod_python
>>> from mod_python import util
>>> def handler(req):
>>> #Removing this line solves the problem.
>>> F=util.FieldStorage( req )
>>> return mod_python.apache.OK
>>> =============================
>>> HTTP Request (dump using TCPWATCH):
>>> =============================
>>> POST http://localhost:80/python/Alertes.py HTTP/1.0
>>> Content-Type: multipart/form-data; boundary=--------061006144341906
>>> Content-Length: 209
>>> Proxy-Connection: keep-alive
>>> Host: www.tx2-localhost
>>> Accept: text/html, */*
>>> User-Agent: Mozilla/3.0 (compatible; Indy Library)
>>> Proxy-Authorization: Basic Og==
>>>
>>> ----------061006144341906
>>> Content-Disposition: form-data; name="TYPE"
>>>
>>> LAST_ALERTS
>>> ----------061006144341906
>>> Content-Disposition: form-data; name="FILEAGE"
>>>
>>> 180
>>>
>>> ----------061006144341906
>
>
Re: memory leaks (was Re: Commented: (MODPYTHON-172) Memory leak
with util.fieldstorage using mod_python 3.2.8 on apache 2.0.55)
Posted by Jim Gallacher <jp...@jgassociates.ca>.
Harold Ship wrote:
> Jim Gallacher <jpg <at> jgassociates.ca> writes:
>
>> I've run some tests to evaluate the memory leaks. The tests are brute
>> force - make requests and watch the memory changes with top -b.
>>
>
> ...
>
>> First up - our leaky 3.2.9, and man does it leak! The readlines request
>> has a body of 1M bytes, and it fails in a hurry.
>>
>
> ...
>
>> Everything is looking much better, with readlines looking very stable.
>> Cfgtree_walk and parse_qsl are also looking much better but there may
>> still be a problem. To confirm this I ran the tests against 3.2.x build
>> 20060709 again, but with 100,000 requests.
>>
>
> Did you have a chance to re-run the tests?
I ran my test using FieldStorage but haven't had a chance to go further.
For 500k requests memory consumption increases from 1.1% to 10%
(mpm-worker, StartServers = 2) so it still leaks. This is still an
improvement as the machine would have crumpled far before the 500k
request mark, but a leak is a leak. I assume the main culprit in
FieldStorage is parse_qsl, but I don't see any obvious problem.
> There are some things I've found, although I don't think they cause a leak.
> For example, in parse_qs() there is:
>
> pair = PyString_FromStringAndSize(NULL, len);
> if (pair == NULL)
> return NULL;
>
> this should be:
>
> pair = PyString_FromStringAndSize(NULL, len);
> if (pair == NULL) {
> Py_DECREF(pairs);
> return NULL;
> }
>
> There are a few more like that in parse_qs() and parse_qsl(), and cfgtree_walk().
>
> In req_readlines(), line 810/811 the intention isn't clear of:
>
> if (result == NULL)
> return PyErr_NoMemory();
I agree it would be clearer if the result object creation was
immediately before the test. eg
PyObject *result = PyList_New(0);
...
result = PyList_New(0);
if (result == NULL)
return PyErr_NoMemory()
> While the exact same code on lines 814/815 are not correct.
> They should be:
>
> if (rlargs == NULL) { // note rlargs not result
> Py_DECREF(result);
> return PyErr_NoMemory();
> }
(Good catch on the rlargs thing).
I think these are mainly just coding inconsistencies, and agree that
they are should not be the cause of the current memory leaks. If one of
the object creation functions fails due to lack of memory, our failure
to decrement a previously created object is hardly going to matter.
Sloppy programming? - for sure.
The use of PyErr_NoMemory() in the above code fragments does concern me
a little though.
PyErr_NoMemory() is shorthand for PyErr_SetNone(PyExc_MemoryError) and
returns NULL. If a call such as PyList_New(0) fails, doesn't it have the
responsibility to set the exception, and our calling function should
simply return NULL? I had the idea that we should only call
PyErr_NoMemory if we are doing a malloc in our code and it fails. Is
there something unique about PyList_New and PyTuple_New that I'm missing?
By the way Harold, it would be helpful if when talking about specific
code lines of code if you were referencing the current development in
svn trunk. Generally we've been making changes in trunk, and backporting
to branches/3.2.x as required. Anyway, thanks for your continuing
efforts. :)
Jim
Re: memory leaks (was Re: Commented: (MODPYTHON-172) Memory leak with util.fieldstorage using mod_python 3.2.8 on apache 2.0.55)
Posted by Harold Ship <ha...@giant-steps-networks.com>.
Jim Gallacher <jpg <at> jgassociates.ca> writes:
>
> I've run some tests to evaluate the memory leaks. The tests are brute
> force - make requests and watch the memory changes with top -b.
>
...
> First up - our leaky 3.2.9, and man does it leak! The readlines request
> has a body of 1M bytes, and it fails in a hurry.
>
...
> Everything is looking much better, with readlines looking very stable.
> Cfgtree_walk and parse_qsl are also looking much better but there may
> still be a problem. To confirm this I ran the tests against 3.2.x build
> 20060709 again, but with 100,000 requests.
>
Did you have a chance to re-run the tests?
There are some things I've found, although I don't think they cause a leak.
For example, in parse_qs() there is:
pair = PyString_FromStringAndSize(NULL, len);
if (pair == NULL)
return NULL;
this should be:
pair = PyString_FromStringAndSize(NULL, len);
if (pair == NULL) {
Py_DECREF(pairs);
return NULL;
}
There are a few more like that in parse_qs() and parse_qsl(), and cfgtree_walk().
In req_readlines(), line 810/811 the intention isn't clear of:
if (result == NULL)
return PyErr_NoMemory();
While the exact same code on lines 814/815 are not correct.
They should be:
if (rlargs == NULL) { // note rlargs not result
Py_DECREF(result);
return PyErr_NoMemory();
}