Crash in freethreading when acquiring/releasing the GIL in a finalizer

[da-woods]

Bug report

Bug description:

bug.cpp

#include <thread>
#include <cstdio>
#include <latch>
#include <Python.h>

PyObject* callInDestructor(PyObject *self, PyObject *args) {
    auto state = PyGILState_Ensure();
    printf("In destructor\n");
    PyGILState_Release(state);
    Py_RETURN_NONE;
}

PyObject *doStuff(PyObject *self, PyObject *cls) {
    PyObject *o;
    Py_BEGIN_ALLOW_THREADS  // I'm not really sure why this is needed...
    {
        std::latch l1(1);
        std::latch l2(1);
        std::latch l3(1);
        auto thread1 = std::jthread([&](){
            l1.wait();
            auto state = PyGILState_Ensure();
            o = PyObject_CallNoArgs(cls);
            l2.count_down();
            // printf("0\n");
            l3.wait();
            PyGILState_Release(state);
            printf("thread1 end\n");
        });
        auto thread2 = std::jthread([&](){
            l1.count_down();
            auto state = PyGILState_Ensure();
            l2.wait();
            Py_XDECREF(o);
            l3.count_down();
            PyGILState_Release(state);
            printf("thread2 end\n");
        });
    }
    Py_END_ALLOW_THREADS
    Py_RETURN_NONE;
}

static PyMethodDef methods[] = {
    {"doStuff",  doStuff, METH_O, "Demonstrate error."},
    {"callInDestructor", callInDestructor, METH_NOARGS, "destruct"},
    {NULL, NULL, 0, NULL}        /* Sentinel */
};

static struct PyModuleDef moduleDef = {
    PyModuleDef_HEAD_INIT,
    "bug",   /* name of module */
    NULL,
    -1,       /* size of per-interpreter state of the module,
                 or -1 if the module keeps state in global variables. */
    methods
};

PyMODINIT_FUNC
PyInit_bug(void)
{
    PyObject *m;

    m = PyModule_Create(&moduleDef);
    if (m == NULL)
        return NULL;

    return m;
}

setupbug.py

from setuptools import Extension, setup

setup(
    ext_modules=[
        Extension(
            name="bug",
            sources=["bug.cpp"],
            language="C++",
            extra_compile_args=['-std=c++20'],
            extra_link_args=['-lstdc++'],
        ),
    ]
)

testbug.py

import bug

class C:
    def __del__(self):
        bug.callInDestructor()

bug.doStuff(C)

Build with python setupbug.py build_ext --inplace

Run with python -Xgil=0 testbug.py

I'm a little unclear on exactly what's going wrong here, but essentially it's destroying the C object from within merge_queued_objects(&brc->local_objects_to_merge);, callInDestruction() calls PyGILState_Ensure and PyGILState_Release (which is unnecessary because it already has the GIL, but should be fine anyway), and it's around here that it crashes with a segmentation fault.

If I remove the GIL handing from callInDestruction then it doesn't crash for me.

This is a cut-down version of something I've seen in Cython cython/cython#6214 (comment) with a very similar crash but happening in a slightly different way.

CPython versions tested on:

3.13

Operating systems tested on:

Linux

Linked PRs

• gh-119585: Fix crash involving PyGILState_Release() and PyThreadState_Clear() #119753
• [3.13] gh-119585: Fix crash involving PyGILState_Release() and PyThreadState_Clear() (GH-119753) #119859
• [3.12] gh-119585: Fix crash involving PyGILState_Release() and PyThreadState_Clear() (GH-119753) #119861

[colesbury]

Thanks -- I was able to reproduce the issue easily with your repro. The problem is roughly:

• PyGILState_Release() calls PyThreadState_Clear()
• PyThreadState_Clear() calls some destructors
• The destructors call PyGILState_Ensure() and PyGILState_Release(), but the original PyGILState_Release() is started, but not quite finished so it leads to an invalid state.

PyThreadState_Clear() can call destructors in the default build too, but it happens more often in the free-threaded build.

I think the fix is relatively simple: don't decrement the gilstate_counter until after we call PyThreadState_Clear():

cpython/Python/pystate.c

Line 2801 in a150679

--tstate->gilstate_counter;