I'm trying implement some saner module shutdown procedure (similar to bpo-812369). One of the many problems I've run into is that the GC doesn't know about the m_copy attribute of modules.

I think the attached patch is correct. tp_tranverse exposes m_copy if it is non-NULL. tp_clear calls Py_CLEAR on it.

Oops, my patch doesn't work since m_base can be shared by more than one module instance. I guess a different solution would be to cleanup the m_copy references on interpreter shutdown. Somehow they would have to be found though.

The attached patch seems better. The copies of module dicts stored in the interpreter state are dereferenced when the interpreter shuts down.

Ok, the patch doesn't apply cleanly but looks good regardless.
(although I wonder whether you really reclaim significant stuff here: most C extension dicts should be mostly references to C methods and types)

I will still empty modules_by_index rather than Py_CLEAR it, though.

New changeset 62658d9d8926 by Antoine Pitrou in branch 'default':
Issue bpo-10241: Clear extension module dict copies at interpreter shutdown.
http://hg.python.org/cpython/rev/62658d9d8926

Reopening. This seems to produce segfaults on several of the buildbots, e.g.:
http://buildbot.python.org/all/builders/x86%20XP-4%203.x/builds/8947
http://buildbot.python.org/all/builders/x86%20Windows%20Server%202003%20%5BSB%5D%203.x/builds/1257
http://buildbot.python.org/all/builders/x86%20Gentoo%203.x/builds/4671

I managed to get a core dump locally. Interestingly, it may be that the _tk module is the culprit. The backtrace is the following:

#0  0x00007ff6c5e14aab in raise () from /lib/x86_64-linux-gnu/libpthread.so.0
#1  0x000000000044e57e in faulthandler_fatal_error (signum=11) at ./Modules/faulthandler.c:305
#2  <signal handler called>
#3  0x00000000004c085e in _Py_Dealloc (op=<unknown at remote 0x7ff6bca2ac20>) at Objects/object.c:1873
#4  0x00000000004a8085 in free_keys_object (keys=0x3100ab0) at Objects/dictobject.c:378
#5  0x00000000004ab0e8 in dict_dealloc (mp=0x7ff6be6194b8) at Objects/dictobject.c:1400
#6  0x00000000004c0860 in _Py_Dealloc (
    op={'children': <unknown at remote 0x7ff6bc9ab3d8>, 'master': None, 'tk': <unknown at remote 0x7ff6bca2ac20>, '_tkloaded': 1, '_tclCommands': None}) at Objects/object.c:1873
#7  0x00000000004d7ac7 in subtype_dealloc (
    self=<Tk(children=<unknown at remote 0x7ff6bc9ab3d8>, master=None, tk=<unknown at remote 0x7ff6bca2ac20>, _tkloaded=1, _tclCommands=None) at remote 0x7ff6bc9a7dc0>) at Objects/typeobject.c:1049
#8  0x00000000004c0860 in _Py_Dealloc (
    op=<Tk(children=<unknown at remote 0x7ff6bc9ab3d8>, master=None, tk=<unknown at remote 0x7ff6bca2ac20>, _tkloaded=1, _tclCommands=None) at remote 0x7ff6bc9a7dc0>) at Objects/object.c:1873
#9  0x00000000004a8085 in free_keys_object (keys=0x3100e40) at Objects/dictobject.c:378
#10 0x00000000004aac8f in PyDict_Clear (op={}) at Objects/dictobject.c:1291
#11 0x00000000004dd9c8 in type_clear (type=0x14f45b8) at Objects/typeobject.c:2981
#12 0x000000000043a703 in delete_garbage (collectable=0x7fffab364300, old=0x8de7c0 <generations+64>) at Modules/gcmodule.c:854
#13 0x000000000043ac9d in collect (generation=2, n_collected=0x0, n_uncollectable=0x0, nofail=1) at Modules/gcmodule.c:1020
#14 0x000000000043bfa3 in _PyGC_CollectNoFail () at Modules/gcmodule.c:1619
#15 0x00000000005c6cb6 in PyImport_Cleanup () at Python/import.c:420
#16 0x000000000041d992 in Py_Finalize () at Python/pythonrun.c:552
#17 0x000000000042392b in Py_Exit (sts=0) at Python/pythonrun.c:2474
#18 0x000000000042114b in handle_system_exit () at Python/pythonrun.c:1673
#19 0x0000000000421176 in PyErr_PrintEx (set_sys_last_vars=1) at Python/pythonrun.c:1683
#20 0x0000000000420ddd in PyErr_Print () at Python/pythonrun.c:1590
#21 0x0000000000437fc4 in RunModule (modname=0xf4d570 L"test", set_argv0=1) at Modules/main.c:227
#22 0x0000000000439279 in Py_Main (argc=18, argv=0xf4c020) at Modules/main.c:715
#23 0x00000000004199b5 in main (argc=18, argv=0x7fffab364908) at ./Modules/python.c:69

_tkinter uses PyType_FromSpec() to create its types and then adds them to the module dict without increfing them, so there may be a refcount issue explaining that those types get garbage collected before all their instances die.

New changeset 314a872f54e1 by Antoine Pitrou in branch 'default':
Backout 62658d9d8926 (issue bpo-10241): it causes a crash at shutdown when deallocating a Tkapp object.
http://hg.python.org/cpython/rev/314a872f54e1

I backed out the changeset in time for the Alpha 1 release.
Further investigation should perhaps be made on the tkinter issue, but it's a bit cumbersome: you need a full test run in non-parallel mode to exhibit the segfault.

New changeset 1edff836c954 by Antoine Pitrou in branch 'default':
Issue bpo-10241: Clear extension module dict copies at interpreter shutdown.
http://hg.python.org/cpython/rev/1edff836c954

Re-applied patch after having fixed the tkinter issue.

I'm closing this issue as fixed though I'm not entirely sure the fix is right. The alpha cycle will allow us to get feedback on potential incompatibilities with third-party software.

I'm getting a couple of Valgrind leaks, starting with 1edff836c954:

valgrind --suppressions=Misc/valgrind-python.supp --leak-check=full ./python -c "pass"

All of them seem to be in _PySys_Init().

Can you identify which objects those leaks point to?
The sys module is cleared at shutdown.

One example of a leaked object is "hexversion", from sysmodule.c:1615.
I tried adding it to sys_deletes[] in import.c, but it did not help.

See valgrind.out for other leaks.

IMO we have two references to many newly created values in _PySys_Init():

       SET_SYS_FROM_STRING("hexversion",
                            PyLong_FromLong(PY_VERSION_HEX));

One from PyLong_FromLong() and the other from PyDict_SetItemString() in the
macro. So it may well be that the fix for this issue just exposes the fact
that the last reference is no longer reachable if m_copy is cleared.

See also 31796b188bec.

Does pulling the Py_DECREF before the "if" fix the leaks?

Antoine Pitrou <report@bugs.python.org> wrote:

Does pulling the Py_DECREF before the "if" fix the leaks?

Yes, but I get an additional failure in test_capi. Apparently not all values
require the DECREF. Got to look on a case by case basis.

Valgrind is happy after the recent 5eb00460e6e8.