classification
Title: Leak in atexitmodule
Type: resource usage Stage: resolved
Components: Extension Modules Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, loewis, pitrou, python-dev, skrah
Priority: normal Keywords: patch

Created on 2011-04-11 06:56 by skrah, last changed 2012-03-27 10:40 by skrah. This issue is now closed.

Files
File name Uploaded Description Edit
atexit-leak.patch skrah, 2011-04-11 06:56 review
Messages (6)
msg133501 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2011-04-11 06:56
Valgrind reports a leak (definitely lost) in atexitmodule.c. The
patch fixes the problem.
msg133983 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-04-18 17:17
It's the very first usage of PyModuleDef::m_free.
Martin, do you agree with the path?
msg155742 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-03-14 10:31
Well, if it doesn't crash, it's probably ok ;)
Perhaps check modstate->atexit_callbacks for non-NULL? Or do we trust free() to do the right thing?
msg155883 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-03-15 12:56
Antoine Pitrou <report@bugs.python.org> wrote:
> Well, if it doesn't crash, it's probably ok ;)
> Perhaps check modstate->atexit_callbacks for non-NULL?
> Or do we trust free() to do the right thing?

I was initially surprised by this, but the docs state that it's safe:

http://docs.python.org/dev/c-api/memory.html?highlight=pymem_free#PyMem_Free

The I searched a bit and it appears that free() crashing on NULL is
a pre-ANSI thing.
msg156826 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2012-03-26 15:02
Actually _iomodule.c already has a freefunc with the same signature.
atexit_free() is properly called in:

static void
module_dealloc(PyModuleObject *m)
{
    PyObject_GC_UnTrack(m);
    if (m->md_def && m->md_def->m_free)
        m->md_def->m_free(m);
    if (m->md_dict != NULL) {
        _PyModule_Clear((PyObject *)m);
        Py_DECREF(m->md_dict);
    }
    if (m->md_state != NULL)
        PyMem_FREE(m->md_state);
    Py_TYPE(m)->tp_free((PyObject *)m);
}


So my only worry is if there's a way to exploit the fact that _PyModule_Clear()
is called after atexit_free(). I tried things like: 

>>> import atexit
>>> def g(): pass
...
>>> class silly:
...     def __del__(self): atexit.register(g)
...
>>> atexit.x = silly()
>>> atexit.register(g)
<function g at 0x7fe7ebb83a68>
>>>
Exception AttributeError: "'NoneType' object has no attribute 'register'" in <bound method silly.__del__ of <__main__.silly object at 0x153fc50>> ignored


But I haven't been able to break anything, so I think I'll go ahead and
commit if there aren't any protests.
msg156900 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-03-27 09:50
New changeset 7c48bb929e6e by Stefan Krah in branch 'default':
Issue #11826: Fix memory leak in atexitmodule.
http://hg.python.org/cpython/rev/7c48bb929e6e
History
Date User Action Args
2012-03-27 10:40:16skrahsetstatus: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: - Python 3.2
2012-03-27 09:50:29python-devsetnosy: + python-dev
messages: + msg156900
2012-03-26 15:02:11skrahsetmessages: + msg156826
2012-03-15 12:56:26skrahsetmessages: + msg155883
2012-03-14 10:31:17pitrousetnosy: + pitrou
messages: + msg155742
2011-04-18 17:17:18amaury.forgeotdarcsetnosy: + amaury.forgeotdarc, loewis
messages: + msg133983
2011-04-11 06:56:19skrahcreate