A user reported a segfault when using our mxURL extension with Python 2.7.4. Previous Python versions do not show this behavior, so it's new in Python 2.7.4.

The extension uses an Py_AtExit() function to clean up among other things a reference to a dictionary from another module. The dictionary deallocation then causes the segfault:

$ gdb /usr/local/bin/python2.7
GNU gdb (GDB) 7.0.1-debian
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html
>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i486-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /usr/local/bin/python2.7...done.
(gdb) r
Starting program: /usr/local/bin/python2.7
[Thread debugging using libthread_db enabled]
Python 2.7.4 (default, Apr  8 2013, 15:51:19)
[GCC 4.4.5] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import mx.URL
>>> import sys
>>> sys.exit()

Program received signal SIGSEGV, Segmentation fault.
0x08091201 in dict_dealloc (mp=0xb7b2813c) at Objects/dictobject.c:1005
1005        Py_TRASHCAN_SAFE_BEGIN(mp)
(gdb) bt
#0  0x08091201 in dict_dealloc (mp=0xb7b2813c) at Objects/dictobject.c:1005
#1  0xb7875928 in mxURLModule_Cleanup () at mx/URL/mxURL/mxURL.c:2789
#2  0x0810553f in call_ll_exitfuncs () at Python/pythonrun.c:1763
#3  Py_Finalize () at Python/pythonrun.c:554
#4  0x08104bac in Py_Exit () at Python/pythonrun.c:1772
#5  handle_system_exit () at Python/pythonrun.c:1146
#6  0x0810517d in PyErr_PrintEx (set_sys_last_vars=<value optimized out>)
at Python/pythonrun.c:1156
#7  0x081058dd in PyRun_InteractiveOneFlags (fp=0xb7f8b420,
filename=0x815f9c0 "<stdin>", flags=0xbffffc4c) at Python/pythonrun.c:855
#8  0x08105a68 in PyRun_InteractiveLoopFlags (fp=0xb7f8b420,
filename=0x815f9c0 "<stdin>", flags=0xbffffc4c) at Python/pythonrun.c:772
#9  0x081062f2 in PyRun_AnyFileExFlags (fp=0xb7f8b420, filename=0x815f9c0
"<stdin>", closeit=0, flags=0xbffffc4c) at Python/pythonrun.c:741
#10 0x0805bb59 in Py_Main (argc=1, argv=0xbffffd34) at Modules/main.c:640
#11 0x0805abeb in main (argc=1, argv=0xbffffd34) at ./Modules/python.c:23
(gdb)

After a closer look at recent checkins, I found this checking for the trash can mechanism: 5a2ef447b80d (ticket #13992).

This appears to be the cause:

    1.20  #define Py_TRASHCAN_SAFE_BEGIN(op) \
    1.21 -    if (_PyTrash_delete_nesting < PyTrash_UNWIND_LEVEL) { \
    1.22 -        ++_PyTrash_delete_nesting;
    1.23 -        /* The body of the deallocator is here. */
    1.24 +    do { \
    1.25 +        PyThreadState *_tstate = PyThreadState_GET(); \
    1.26 +        if (_tstate->trash_delete_nesting < PyTrash_UNWIND_LEVEL) { \
    1.27 +            ++_tstate->trash_delete_nesting;
    1.28 +            /* The body of the deallocator is here. */

At the time the Py_AtExit functions are called, the thread state
is NULL, so the if (_tstate->...) segfaults.

I think a solution to the problem would be to check _tstate for NULL and only use it if it is non-NULL - without threads you don't need to keep track of them ;-)

> At the time the Py_AtExit functions are called, the thread state is NULL

I'd say this is the root cause. It's a bad thing to call Py_DECREF without a thread state.
Was it the case in previous versions?

On 12.04.2013 16:00, Amaury Forgeot d'Arc wrote:
> 
> Amaury Forgeot d'Arc added the comment:
> 
>> At the time the Py_AtExit functions are called, the thread state is NULL
> 
> I'd say this is the root cause. It's a bad thing to call Py_DECREF without a thread state.
> Was it the case in previous versions?

The extension has been using this ever since it was written many
years ago and without any problems in all Python versions up, but not
including 2.7.4. It's the only way to do module cleanup in Python 2.x.

So far, we've only seen the problem for dictionaries that
are DECREFed.

I know that things may go wrong when DECREF'ing objects this late in
the finalization process, but still expect Python to handle errors
gracefully without segfaulting, as it has been the case for all
previous Python versions.

Judging by the fact that the Py_AtExit funcs are called at the very end of Py_Finalize (after absolutely everything has been cleaned up), I think you shouldn't use any Python facilities at this point.

By the way, why do you try to clean up the dictionary at this point? Any non-trivial deallocation code (e.g. __del__ method) will fail running.

On 12.04.2013 17:32, Antoine Pitrou wrote:
> 
> Judging by the fact that the Py_AtExit funcs are called at the very end of Py_Finalize (after absolutely everything has been cleaned up), I think you shouldn't use any Python facilities at this point.

As mentioned earlier: this is the only way to cleanup extension modules
in Python 2.x and the trash can patch broke this.

> By the way, why do you try to clean up the dictionary at this point? Any non-trivial deallocation
code (e.g. __del__ method) will fail running.

The dictionaries just contain simple Python types that don't
have __del__ methods, so deallocation works fine and it prevents
memory leaks when embedding the Python interpreter and restarting
it several times.

> As mentioned earlier: this is the only way to cleanup extension
> modules
> in Python 2.x and the trash can patch broke this.

Well, the doc clearly says
"Since Python’s internal finalization will have completed before
the cleanup function, no Python APIs should be called by func".

Why wouldn't you clean up the dict (if it exists) at module
initialization instead? You will have a working Python
runtime at this point.

That said, we could hack a 2.7-specific hack to the patch, in
order to disable the trashcan mechanism at shutdown.

Marc-André, does this patch work for you?

On 12.04.2013 20:00, Antoine Pitrou wrote:
> 
> Marc-André, does this patch work for you?
> 
> Added file: http://bugs.python.org/file29791/tstate_trashcan.patch

Thanks, Antoine. I can try this tomorrow.

Yes, let's not break thing in point releases.

Checked the patch: it fixes the problem. Thanks.

Will this go into Python 2.7.5 ?

I'm asking because we need to issue a patch level release of egenix-mx-base and if Python 2.7.5 will fix the problem, we'll just add the work-around for Python 2.7.4.

Regarding a better cleanup logic: This is available in Python 3.x with the new module init logic and we'll use it there.

PS: The approach with doing the cleanup at module startup time won't work, because the still existing objects will reference parts of the already cleaned up interpreter instance.

New changeset e814fbd470bf by Antoine Pitrou in branch '2.7':
Issue #17703: Fix a regression where an illegal use of Py_DECREF() after interpreter finalization can cause a crash.
http://hg.python.org/cpython/rev/e814fbd470bf

Committed!

On 15.04.2013 21:21, Antoine Pitrou wrote:
> 
> Committed!

Cool, thanks for the quick turnaround.

It does still cause a Fatal Error in debug build as PyThreadState_GET() error out if _PyThreadState_Current is NULL

Guillaume already mentioned this, its still causing a Fatal Error. To fix this PyThreadState_GET() in Py_TRASHCAN_SAFE_BEGIN must be replaced with _PyThreadState_Current

#define Py_TRASHCAN_SAFE_BEGIN(op) \
    do { \
        PyThreadState *_tstate = _PyThreadState_Current; \

I'm seeing this bug in Python 3.4.2 as well, and the patch here (tstate_trashcan.patch) appears to fix it. I'm using boost 1.57.0, and Python was compiled on a vanilla rhel6 system.

Can anyone else confirm this bug in 3.4?

> I'm seeing this bug in Python 3.4.2 as well, and the patch here (tstate_trashcan.patch) appears to fix it.

What is the context? Some specific C code?

I can verify this bug does still in fact exist in all 3.x versions I've tested (3.4-3.7).

It's been the cause of many long-standing segfaults we've been having at exit in applications that use OpenRAVE and/or boost::python::numeric.

I can also confirm that the included patch does "workaround" the issue.

A big pain point here for me is in determining what exit handler is causing this.

GDB has been it's usual great help in determining information about the exact segfault, but the problem code itself is rather elusive since it's ran so much earlier and just causes this issue downstream later in execution. Unfortunately the extensions I'm dealing with also happen to take a lifetime to compile, exacerbated by the fact that they are dependent upon one another.

Can we please apply this to 3.x's tree?

This issue is closed. Can you please open a new issue? Try to describe the bug that you have and what you attemped to fix it.

See also https://bugs.python.org/issue35983 for another trashcan-related issue.