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

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.

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);

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

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.

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>
> _______________________________________
>

> 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.

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.

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

Closing as the original issue is fixed. Other leaks should be reported as separate issues.