classification
Title: Embedded python, handle (memory) leak
Type: resource usage Stage: resolved
Components: Interpreter Core Versions: Python 3.2, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: StevenY, loewis, martind, pitrou, python-dev, skrah
Priority: normal Keywords: patch

Created on 2010-11-08 17:38 by martind, last changed 2013-05-04 18:54 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
freelocks.patch pitrou, 2011-10-27 22:39
Messages (10)
msg120782 - (view) Author: Martin Dunschen (martind) Date: 2010-11-08 17:38
I found a number of 'handle leaks' in the core code to embed python into a C/C++ application on windows. 
To reproduce: 
The simplest possible embedded application only calls:

#include <Python.h>
void entry()
{
    Py_Initialize();
    Py_Finalize();
}

I found (but this does not seem important to the problem) that when the above code is compiled into a Windows DLL, and from another simple app this DLL is loaded, the entry function is called and then the DLL is unloaded, the number of handles held by the application is increasing (by 3). Calling the steps "LoadLibrary, entry, FreeLibrary" in a loop will increase the number of held handles by 3 every time.

I can propose fixes for these 3 leaks, please find attached in the zip file patches for the files I have fixed.

But there are some issues remaining:
PyEval_InitThreads
allocates 'interpreter_lock', but there is nothing in the API that allows you to free this lock again. Should there maybe a function, in the API

void PyEval_FinishThreads(void)
{
    if (!interpreter_lock)
	return;
    PyThread_free_lock(interpreter_lock);
}

That would get rid of another handle leak if a DLL embedding python would use PyEval_InitThreads.

In a specific DLL I am debugging just now I observe 2 more handle leaks (The 4 I report here, and 2 more I have not yet found).

I hope my report is comprehensive and can be reproduced. I am happy to be of assistance if there are any questions.

Martin
msg120856 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2010-11-09 10:36
I don't see a file attached. Could you attach your patches in plain text?
It is the preferred method, since files can be easily viewed in the browser.
msg120862 - (view) Author: Martin Dunschen (martind) Date: 2010-11-09 12:35
Here my suggested changes in plain text (I generated these as patches from a diff to the current code):

thread.c:
353a354,359
> void PyThread_fini()
> {
> 	// should assert here that nkeys == 0
> 	PyThread_free_lock(keymutex);
> }
> 


pystate.c:
38a39
> #define HEAD_RELEASE() PyThread_free_lock(head_mutex);
48a50
> #define HEAD_RELEASE() /* Nothing */
140a143
> 	HEAD_RELEASE();
422a426,427
> void PyThread_fini(); // forward
> 
428c433,434
< 	autoInterpreterState = NULL;;
---
> 	autoInterpreterState = NULL;
> PyThread_fini(); // this frees a lock called keymutex in thread.c


import.c
473a474,475
> 
> 	PyThread_free_lock(import_lock);
msg120919 - (view) Author: Martin Dunschen (martind) Date: 2010-11-10 14:42
I have identified 5 cases where objects of type PyThread_type_lock are allocated but never freed again:

import.c:
    import_lock

pystate.c:
    head_mutex

thread.c:
    key_mutex

ceval.c:
    interpreter_lock

zliblock.c:
    zlib_lock

This leads to a handle leak on windows (PyThread_type_lock) is mapped to PNRMUTEX, a structure that contains a HANDLE to an event, created by CreateEvent, but never released with CloseHandle. As I can see it, common to all thread implementations (in thread.c) is the PyThread_allocate_lock call for the above locks, but it is not matched by a PyThread_free_lock. I can imagine there are similar memory leaks in other thread implementations for different OS.

Additionally there is one directly created event in
timemodule.c
    hInterruptEvent

And finally in
myreadline.c:
    _PyOS_ReadmeLock

no matching PyThread_free_lock for the call to PyThread_allocate_lock.


All these memory or handle leaks could easily be fixed by using some C++ smart pointer implementations without requiring any changes to the API for embedding/extending python. On windows it requires some minor changes to allow compiling thread.c and timemodule.c as C++ code.

I am happy to post my version here, for other more knowledgeable python programmers to review them. I see my suggestions as a patch, but in an ideal world a lot of the code in pythoncore could be reimplemented using proper C++.

Martin
msg146528 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-10-27 22:39
Here is a patch that fixes some of these handle leaks in Python 3.2.
However, as a general guideline, you shouldn't unload the Python DLL if you fish to use it later again. Just keep it in memory (the DLL isn't very big, is it?). Yes, C++ would allow to solve this, but the interpreter is currently written in C and there's no plan, even in the middle term, to change this.
msg146543 - (view) Author: Martin Dunschen (martind) Date: 2011-10-28 09:08
Hello Antoine

Unloading would not be necessary if the DLL is just the python interpreter,
but if you build a DLL with python embedded that does quite a bit more than
some python interpreting (in my case complex C/C++ numerical calculations)
unloading the DLL is a convenient way to free up unused resources. There are
C++ classes that extend python in my code (via swig), and that's where the
size of the DLL get's increased.

If the handle leaks are restricted to the windows implementation of cpython,
could it not be justified to allow C++ in a patch, I can't think of a C only
compiler for windows?

Thanks

Martin

On Thu, Oct 27, 2011 at 11:39 PM, Antoine Pitrou <report@bugs.python.org>wrote:

>
> Antoine Pitrou <pitrou@free.fr> added the comment:
>
> Here is a patch that fixes some of these handle leaks in Python 3.2.
> However, as a general guideline, you shouldn't unload the Python DLL if you
> fish to use it later again. Just keep it in memory (the DLL isn't very big,
> is it?). Yes, C++ would allow to solve this, but the interpreter is
> currently written in C and there's no plan, even in the middle term, to
> change this.
>
> ----------
> keywords: +patch
> nosy: +loewis
> Added file: http://bugs.python.org/file23539/freelocks.patch
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue10363>
> _______________________________________
>
msg146610 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-10-29 14:25
> If the handle leaks are restricted to the windows implementation of cpython,
> could it not be justified to allow C++ in a patch, I can't think of a C only
> compiler for windows?

Well, I think that would be rather clumsy. I'm not a Windows user myself, perhaps other people can share opinions.
msg146615 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2011-10-29 17:41
As a policy, we will not rely on C++ destructors for cleanup. There are really two issues here: "global" locks, and module-specific locks.

The global locks can and should be released in Py_Finalize, with no API change. Antoine's patch looks good to me.

For module-level locks, PEP-3121-style module finalization should be used. Patches are welcome.

Martin: Please understand that there are *MANY* more issues with reloading Python in the same process, as a really large number of stuff doesn't get cleanup up on shutdown. Also expect that fixing these will take 10 years or more, unless somebody puts a huge effort into it.
msg146651 - (view) Author: Roundup Robot (python-dev) Date: 2011-10-30 18:19
New changeset 608975eafe86 by Antoine Pitrou in branch '3.2':
Issue #10363: Deallocate global locks in Py_Finalize().
http://hg.python.org/cpython/rev/608975eafe86

New changeset 728595c16acd by Antoine Pitrou in branch 'default':
Issue #10363: Deallocate global locks in Py_Finalize().
http://hg.python.org/cpython/rev/728595c16acd
msg188398 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-04 18:54
Closing as the original issue is fixed. Other leaks should be reported as separate issues.
History
Date User Action Args
2013-05-04 18:54:44pitrousetstatus: open -> closed
resolution: fixed
messages: + msg188398

stage: resolved
2011-10-30 18:19:54python-devsetnosy: + python-dev
messages: + msg146651
2011-10-29 17:41:30loewissetmessages: + msg146615
2011-10-29 14:25:13pitrousetmessages: + msg146610
2011-10-28 09:08:28martindsetmessages: + msg146543
2011-10-27 22:39:22pitrousetfiles: + freelocks.patch

nosy: + loewis
messages: + msg146528

keywords: + patch
2011-10-24 01:25:36pitrousetnosy: + pitrou

versions: + Python 3.2, Python 3.3, - Python 2.7
2010-12-03 12:12:33StevenYsetnosy: + StevenY
2010-11-10 14:42:47martindsetmessages: + msg120919
2010-11-09 12:35:27martindsetmessages: + msg120862
2010-11-09 10:36:14skrahsetnosy: + skrah
messages: + msg120856
2010-11-08 22:05:22loewissetversions: - Python 2.6, Python 2.5
2010-11-08 17:38:46martindcreate