Skip to content

Segmentation fault during interpreter finalization in PyTraceMalloc_Untrack #129185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rostan-t opened this issue Jan 22, 2025 · 3 comments
Closed
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@rostan-t
Copy link

rostan-t commented Jan 22, 2025

Crash report

What happened?

_PyTraceMalloc_Fini is called before _PyImport_Fini in _Py_Finalize. This can lead to crashes when using tracemalloc if calls to PyTraceMalloc_Untrack are performed during finalization of module (e.g in the destructor of some global variable).

Commit 6b47499 moved the call to TABLES_LOCK before the check tracemalloc_config.tracing so now the issue whether or not tracemalloc is started.

The following extension module reproduces the issue:

#define PY_SSIZE_T_CLEAN
#include <Python.h>
#include <numpy/arrayobject.h>

static PyObject* arr = NULL;

static struct PyModuleDef repromodule = {
    PyModuleDef_HEAD_INIT,
    "repro",
    NULL,
    -1,
    NULL
};

PyMODINIT_FUNC
PyInit_repro(void)
{
    PyObject* m;

    m = PyModule_Create(&repromodule);
    if (!m)
        return NULL;

    import_array();

    arr = PyArray_SimpleNew(1, (npy_intp[]){0}, NPY_DOUBLE);
    if (!arr || PyModule_AddObject(m, "arr", arr) < 0) {
        Py_XDECREF(arr);
        Py_DECREF(m);
        return NULL;
    }

    return m;
}
$ python -c 'import repro'

It declares a global NumPy array and array_dealloc calls PyTraceMalloc_Untrack, creating a segfault.

For context, I initially ran into the issue in a pybind11 extension declaring functions with numpy array as defaults arguments.

CPython versions tested on:

CPython main branch, 3.13

Operating systems tested on:

Linux

Output from running 'python -VV' on the command line:

Python 3.13.1+ (heads/3.13:a3797492179, Jan 22 2025, 11:46:15) [GCC 11.4.0]

Linked PRs

@rostan-t rostan-t added the type-crash A hard crash of the interpreter, possibly with a core dump label Jan 22, 2025
@picnixz picnixz added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Jan 22, 2025
@picnixz
Copy link
Member

picnixz commented Jan 22, 2025

Btw, PyModule_AddObject steals a reference, so you might want to incref arr before since you're storing it in a global variable but I don't know whether this would solve the issue. Now, can you actually produce a reproducer that does not depend on numpy so that we can possibly add a regression test?

cc @vstinner

vstinner added a commit to vstinner/cpython that referenced this issue Jan 22, 2025
Support calling PyTraceMalloc_Track() and PyTraceMalloc_Untrack()
during late Python finalization.
vstinner added a commit to vstinner/cpython that referenced this issue Jan 22, 2025
Support calling PyTraceMalloc_Track() and PyTraceMalloc_Untrack()
during late Python finalization.
@vstinner
Copy link
Member

I wrote a different fix with a test: PR gh-129191.

vstinner added a commit that referenced this issue Jan 23, 2025
Support calling PyTraceMalloc_Track() and PyTraceMalloc_Untrack()
during late Python finalization.

* Call _PyTraceMalloc_Fini() later in Python finalization.
* Test also PyTraceMalloc_Untrack() without the GIL
* PyTraceMalloc_Untrack() now gets the GIL.
* Test also PyTraceMalloc_Untrack() in test_tracemalloc_track_race().
vstinner added a commit to vstinner/cpython that referenced this issue Jan 23, 2025
…29191)

Support calling PyTraceMalloc_Track() and PyTraceMalloc_Untrack()
during late Python finalization.

* Call _PyTraceMalloc_Fini() later in Python finalization.
* Test also PyTraceMalloc_Untrack() without the GIL
* PyTraceMalloc_Untrack() now gets the GIL.
* Test also PyTraceMalloc_Untrack() in test_tracemalloc_track_race().

(cherry picked from commit 46c7e13)
vstinner added a commit to vstinner/cpython that referenced this issue Jan 23, 2025
Always build tracemalloc with PyMem_RawMalloc() hooks.
vstinner added a commit that referenced this issue Jan 23, 2025
Always build tracemalloc with PyMem_RawMalloc() hooks.
vstinner added a commit that referenced this issue Jan 23, 2025
#129217)

gh-129185: Fix PyTraceMalloc_Untrack() at Python exit (#129191)

Support calling PyTraceMalloc_Track() and PyTraceMalloc_Untrack()
during late Python finalization.

* Call _PyTraceMalloc_Fini() later in Python finalization.
* Test also PyTraceMalloc_Untrack() without the GIL
* PyTraceMalloc_Untrack() now gets the GIL.
* Test also PyTraceMalloc_Untrack() in test_tracemalloc_track_race().

(cherry picked from commit 46c7e13)
@vstinner
Copy link
Member

Thanks @rostan-t for the bug report and for the initial fix! It's now fixed in 3.13 and main branches, and the 3.12 fix is incoming.

vstinner added a commit that referenced this issue Jan 23, 2025
#129217) (#129221)

[3.13] gh-129185: Fix PyTraceMalloc_Untrack() at Python exit (#129191) (#129217)

gh-129185: Fix PyTraceMalloc_Untrack() at Python exit (#129191)

Support calling PyTraceMalloc_Track() and PyTraceMalloc_Untrack()
during late Python finalization.

* Call _PyTraceMalloc_Fini() later in Python finalization.
* Test also PyTraceMalloc_Untrack() without the GIL
* PyTraceMalloc_Untrack() now gets the GIL.
* Test also PyTraceMalloc_Untrack() in test_tracemalloc_track_race().

(cherry picked from commit 46c7e13)
(cherry picked from commit e3b3e01)
vstinner added a commit to vstinner/cpython that referenced this issue Jan 23, 2025
vstinner added a commit to vstinner/cpython that referenced this issue Jan 23, 2025
vstinner added a commit to vstinner/cpython that referenced this issue Jan 24, 2025
Since tracemalloc uses PyMutex, it becomes safe to use TABLES_LOCK()
even after _PyTraceMalloc_Fini(): remove the "pre-check" in
PyTraceMalloc_Track() and PyTraceMalloc_Untrack().

PyTraceMalloc_Untrack() no longer needs to acquire the GIL.

_PyTraceMalloc_Fini() can be called earlier during Python
finalization.
vstinner added a commit to vstinner/cpython that referenced this issue Jan 24, 2025
Since tracemalloc uses PyMutex, it becomes safe to use TABLES_LOCK()
even after _PyTraceMalloc_Fini(): remove the "pre-check" in
PyTraceMalloc_Track() and PyTraceMalloc_Untrack().

PyTraceMalloc_Untrack() no longer needs to acquire the GIL.

_PyTraceMalloc_Fini() can be called earlier during Python
finalization.
vstinner added a commit that referenced this issue Jan 24, 2025
Since tracemalloc uses PyMutex, it becomes safe to use TABLES_LOCK()
even after _PyTraceMalloc_Fini(): remove the "pre-check" in
PyTraceMalloc_Track() and PyTraceMalloc_Untrack().

PyTraceMalloc_Untrack() no longer needs to acquire the GIL.

_PyTraceMalloc_Fini() can be called earlier during Python
finalization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Development

No branches or pull requests

3 participants