Re: [jira] Created: (MODPYTHON-184) Memory leak apache.table()
Justin Erenkrantz wrote .. > On 8/16/06, Jim Gallacher <[EMAIL PROTECTED]> wrote: > > Actually I don't think apr_pool_destroy() in table_dealloc is actually > > destroying the pool. I've been poking around in the code and there is > > something odd going on here. > > I would actually love to test this, but I can't build trunk on Mac OS X. Huh! Do you have more than one version of Python installed? I do all my work on Mac OS X and have no problem. I only have the standard OSE supplied version of Python installed though. FWIW, am using Mac OS X 10.4.7 at present. Because of other stuff, have been ignoring the table object issues, but maybe I should start paying more attention. :-) Graham
Re: [jira] Created: (MODPYTHON-184) Memory leak apache.table()
On 8/16/06, Jim Gallacher <[EMAIL PROTECTED]> wrote: Actually I don't think apr_pool_destroy() in table_dealloc is actually destroying the pool. I've been poking around in the code and there is something odd going on here. I would actually love to test this, but I can't build trunk on Mac OS X. The definitions of PyLFS (LINKFORSHARED) refer to values that I don't have: LINKFORSHARED= -u __dummy -u _PyMac_Error -framework System $(PYTHONFRAMEWORKDIR)/Versions/$(VERSION)/$(PYTHONFRAMEWORK) -framework CoreServices -framework Foundation That's not the right line. It should be looking at LDSHARED and PYTHONFRAMEWORK instead. Ideally, we should switch to a format like what Subversion uses to detect the values: which queries Python directly. See: http://svn.collab.net/repos/svn/trunk/build/get-py-info.py The sed magic going on in autoconf is just too creaky. (Subversion builds against Python on Mac OS X just fine.) If we import get-py-info.py to MP, we should prepend it with the license block from: http://svn.collab.net/repos/svn/trunk/subversion/LICENSE (That script doesn't have a license block, but it really should. I'll poke other SVN devs about fixing that, but importing that LICENSE block is a safe compromise in the meantime.) Thanks. -- justin
Re: [jira] Created: (MODPYTHON-184) Memory leak apache.table()
Alexis Marrero wrote:
> Jim,
>
> This are my results for the memory leak search in apache.table().
>
> The table object creates a memory pool by using apr_pool_create_ex() and
> destroys the pool using apr_pool_destroy(). I added a line in
> MpTable_New() before "return (PyObject*)t" to destroy the pool and ran
> 1M iterations and I notice that there was no memory leak. Therefore the
> apache functions seems to be working fine.
Actually I don't think apr_pool_destroy() in table_dealloc is actually
destroying the pool. I've been poking around in the code and there is
something odd going on here.
I tried registering a cleanup in MpTable_New() using:
apr_pool_cleanup_register(t->pool,
"pool cleanup called",
cleanup_test,
apr_pool_cleaunp_null);
The cleanup_test callback just logs the "pool cleanup called" message to
a file.
apr_pool_destroy() is getting called in table_dealloc, but cleanup_test
never gets called which indicates that the pool is *not* being
destroyed, and hence our memory leak.
I tried your trick of immediately calling apr_pool_destroy in
MpTable_New(), and cleanup_test does get called there.
So, the big question is... why is the pool not being destroyed?
Can anyone offer some insight here?
The attached diff is for trunk if anyone wants to play around with it.
Jim
Index: tableobject.c
===
--- tableobject.c (revision 431994)
+++ tableobject.c (working copy)
@@ -59,6 +59,19 @@
return (PyObject *)result;
}
+
+
+apr_status_t cleanup_test(void *msg)
+{
+FILE *f;
+f = fopen("/tmp/debug_table.log", "a+");
+fprintf(f, "%s\n", (char *)msg);
+fclose(f);
+
+return 0;
+}
+
+
/**
** MpTable_New
**
@@ -78,6 +91,8 @@
tableobject *t;
apr_pool_t *p;
+cleanup_test("MpTable_New() called");
+
/* XXX need second arg abort function to report mem error */
apr_pool_create_ex(&p, NULL, NULL, NULL);
@@ -86,7 +101,12 @@
/* remember the pointer to our own pool */
t->pool = p;
+apr_pool_cleanup_register(p, " pool cleanup called", cleanup_test,
apr_pool_cleanup_null);
+/* Uncomment this to test that cleanup_test is getting called correctly.
+apr_pool_destroy(t->pool);
+*/
+
return (PyObject *)t;
}
@@ -99,10 +119,13 @@
static void table_dealloc(register tableobject *self)
{
+cleanup_test("table_dealloc:");
if (MpTable_Check(self)) {
-if (self->pool)
+if (self->pool) {
+cleanup_test(" preparing to destroy the pool");
apr_pool_destroy(self->pool);
+}
PyObject_Del(self);
}
else
Re: [jira] Created: (MODPYTHON-184) Memory leak apache.table()
Alexis Marrero wrote:
> Jim,
>
> This are my results for the memory leak search in apache.table().
>
> The table object creates a memory pool by using apr_pool_create_ex() and
> destroys the pool using apr_pool_destroy(). I added a line in
> MpTable_New() before "return (PyObject*)t" to destroy the pool and ran
> 1M iterations and I notice that there was no memory leak. Therefore the
> apache functions seems to be working fine.
That's weird though, because I stuffed some printf's into table_dealloc,
and apr_pool_destroy(self->pool) is getting called.
> I couldn't fix the problem but here is a work around. In
> mod_python/util.py instead of using apache.make_table() use a regular
> Python dictionary. So the line that looks like:
>
> headers = apache.make_table()
>
> now looks like:
>
> headers = {}
>
> The apache table is basically used a Python dictionary. The only
> functionality that is lost is that apache tables are case insensitive,
> and that can be easily fixed by creating a class in Python that inherits
> from dict type and override the __getitem__ and __setitem__ methods.
I had considered the idea of using a standard python dict, but there is
one other difference that may be important. Apache tables (and therefore
the mp_table wrapper) can have duplicate keys. I'd rather we didn't
create a whole new class for use in FieldStorage.
> For the moment I'm going to keep this changes until modpython.org
> release a patch. I spent quite sometime trying to investigate and solve
> the memory leak problem but the best I was able to do was to work around
> it.
This workaround may be fine for your application where you control what
is being POSTed, but I'm not so sure about it as a general solution as
the change may break other apps.
> BTW, apache.table, apache.make_table or _apache.table is only being
> used in mod_python/util.py.
True, but the mp_table wrapper is used extensively within mod_python,
but these table instances are created using MpTable_FromTable() rather
than MpTable_New(). I'd like to understand why we are getting leak with
MpTable_New(), as it may point to some other bug in mp_table.
BTW, thanks for the time you've spent digging into these leaks.
Jim
>
> /amn
>
> On Aug 13, 2006, at 12:01 PM, Jim Gallacher (JIRA) wrote:
>
>> Memory leak apache.table()
>> --
>>
>> Key: MODPYTHON-184
>> URL: http://issues.apache.org/jira/browse/MODPYTHON-184
>> Project: mod_python
>> Issue Type: Bug
>> Components: core
>> Affects Versions: 3.2.10, 3.3
>> Reporter: Jim Gallacher
>> Assigned To: Jim Gallacher
>>
>>
>> There is a memory leak in apache.table().
>>
>> from mod_python import apache
>>
>> def handler(req):
>> req.content_type = 'text/plain'
>> t = apache.make_table()
>>
>> req.write('ok table:')
>> return apache.OK
>>
>> Using mpm-worker with StartServers 2, and 2 requests results in
>> memory consumption going from 1.2% to 9.3% per process. (ie approx 8k
>> per request)
>>
>>
>>
>>
>> --This message is automatically generated by JIRA.
>> -
>> If you think it was sent incorrectly contact one of the
>> administrators: http://issues.apache.org/jira/secure/Administrators.jspa
>> -
>> For more information on JIRA, see: http://www.atlassian.com/software/jira
>>
>>
>
>
Re: [jira] Created: (MODPYTHON-184) Memory leak apache.table()
Jim,This are my results for the memory leak search in apache.table(). The table object creates a memory pool by using apr_pool_create_ex() and destroys the pool using apr_pool_destroy(). I added a line in MpTable_New() before "return (PyObject*)t" to destroy the pool and ran 1M iterations and I notice that there was no memory leak. Therefore the apache functions seems to be working fine.I couldn't fix the problem but here is a work around. In mod_python/util.py instead of using apache.make_table() use a regular Python dictionary. So the line that looks like:headers = apache.make_table()now looks like:headers = {}The apache table is basically used a Python dictionary. The only functionality that is lost is that apache tables are case insensitive, and that can be easily fixed by creating a class in Python that inherits from dict type and override the __getitem__ and __setitem__ methods.For the moment I'm going to keep this changes until modpython.org release a patch. I spent quite sometime trying to investigate and solve the memory leak problem but the best I was able to do was to work around it.BTW, apache.table, apache.make_table or _apache.table is only being used in mod_python/util.py. /amn On Aug 13, 2006, at 12:01 PM, Jim Gallacher (JIRA) wrote:Memory leak apache.table()-- Key: MODPYTHON-184 URL: http://issues.apache.org/jira/browse/MODPYTHON-184 Project: mod_python Issue Type: Bug Components: core Affects Versions: 3.2.10, 3.3 Reporter: Jim Gallacher Assigned To: Jim GallacherThere is a memory leak in apache.table().from mod_python import apachedef handler(req): req.content_type = 'text/plain' t = apache.make_table() req.write('ok table:') return apache.OKUsing mpm-worker with StartServers 2, and 2 requests results in memory consumption going from 1.2% to 9.3% per process. (ie approx 8k per request)-- This message is automatically generated by JIRA.-If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa-For more information on JIRA, see: http://www.atlassian.com/software/jira
