From 8a34d2a5f7dbf7fc45d3d21e49e6d220a52eb5ba Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 15 Jun 2026 14:42:21 +0100 Subject: [PATCH 1/5] [mypyc] Make function wrappers thread-safe on free-threaded builds --- mypyc/lib-rt/function_wrapper.c | 37 +++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/mypyc/lib-rt/function_wrapper.c b/mypyc/lib-rt/function_wrapper.c index 348c3316cd258..2c4135679a8df 100644 --- a/mypyc/lib-rt/function_wrapper.c +++ b/mypyc/lib-rt/function_wrapper.c @@ -59,13 +59,18 @@ static PyMemberDef CPyFunction_members[] = { PyObject* CPyFunction_get_name(PyObject *op, void *context) { (void)context; CPyFunction *func = (CPyFunction *)op; + PyObject *result; + // The critical section makes the lazy init and the incref atomic with + // respect to a concurrent CPyFunction_set_name on free-threaded builds, + // which would otherwise free func_name from under us. + Py_BEGIN_CRITICAL_SECTION(op); if (unlikely(func->func_name == NULL)) { func->func_name = PyUnicode_InternFromString(((PyCFunctionObject *)func)->m_ml->ml_name); - if (unlikely(func->func_name == NULL)) - return NULL; } - Py_INCREF(func->func_name); - return func->func_name; + result = func->func_name; + Py_XINCREF(result); + Py_END_CRITICAL_SECTION(); + return result; } int CPyFunction_set_name(PyObject *op, PyObject *value, void *context) { @@ -77,8 +82,12 @@ int CPyFunction_set_name(PyObject *op, PyObject *value, void *context) { } Py_INCREF(value); + // The critical section serializes the store against a concurrent getter or + // setter on free-threaded builds, preventing a torn store or double-decref. + Py_BEGIN_CRITICAL_SECTION(op); Py_XDECREF(func->func_name); func->func_name = value; + Py_END_CRITICAL_SECTION(); return 0; } @@ -232,12 +241,32 @@ PyObject* CPyFunction_New(PyObject *module, const char *filename, const char *fu PyObject *code = NULL, *op = NULL; bool set_self = false; +#ifdef Py_GIL_DISABLED + // Double-checked locking: the common case (type already created) is a + // lock-free atomic load. Only the first-time initialization takes the + // mutex, which serializes concurrent creators so we don't leak a type or + // race on the store. The release/acquire pairing ensures a thread that + // observes a non-NULL pointer also sees the fully initialized type. + if (!_Py_atomic_load_ptr_acquire(&CPyFunctionType)) { + static PyMutex type_init_mutex = {0}; + PyMutex_Lock(&type_init_mutex); + if (!CPyFunctionType) { + PyTypeObject *type = (PyTypeObject *)PyType_FromSpec(&CPyFunction_spec); + _Py_atomic_store_ptr_release(&CPyFunctionType, type); + } + PyMutex_Unlock(&type_init_mutex); + if (unlikely(!CPyFunctionType)) { + goto err; + } + } +#else if (!CPyFunctionType) { CPyFunctionType = (PyTypeObject *)PyType_FromSpec(&CPyFunction_spec); if (unlikely(!CPyFunctionType)) { goto err; } } +#endif method = CPyMethodDef_New(funcname, func, func_flags, func_doc); if (unlikely(!method)) { From 29e8e80b72e9e8931d1cbfdd2720a9de9ed2f653 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 15 Jun 2026 17:26:00 +0100 Subject: [PATCH 2/5] Fix another race condition --- mypyc/lib-rt/function_wrapper.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/mypyc/lib-rt/function_wrapper.c b/mypyc/lib-rt/function_wrapper.c index 2c4135679a8df..ce3aade4c0ced 100644 --- a/mypyc/lib-rt/function_wrapper.c +++ b/mypyc/lib-rt/function_wrapper.c @@ -29,7 +29,16 @@ static void CPyFunction_dealloc(CPyFunction *m) { } static PyObject* CPyFunction_repr(CPyFunction *op) { - return PyUnicode_FromFormat("", op->func_name, (void *)op); + // Take a strong ref to the name via the getter, which guards against a + // concurrent CPyFunction_set_name freeing func_name from under us on + // free-threaded builds. It also handles lazy initialization of func_name. + PyObject *name = CPyFunction_get_name((PyObject *)op, NULL); + if (unlikely(name == NULL)) { + return NULL; + } + PyObject *result = PyUnicode_FromFormat("", name, (void *)op); + Py_DECREF(name); + return result; } static PyObject* CPyFunction_call(PyObject *func, PyObject *args, PyObject *kw) { From 6f603a548d5fc621cd93fbc2ef3de56531b2de7c Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 15 Jun 2026 17:30:22 +0100 Subject: [PATCH 3/5] Fix another concurrency bug --- mypyc/lib-rt/function_wrapper.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/mypyc/lib-rt/function_wrapper.c b/mypyc/lib-rt/function_wrapper.c index ce3aade4c0ced..47495d50e2b82 100644 --- a/mypyc/lib-rt/function_wrapper.c +++ b/mypyc/lib-rt/function_wrapper.c @@ -91,12 +91,18 @@ int CPyFunction_set_name(PyObject *op, PyObject *value, void *context) { } Py_INCREF(value); - // The critical section serializes the store against a concurrent getter or - // setter on free-threaded builds, preventing a torn store or double-decref. + // Store the new name and capture the old one under the critical section, + // which serializes the store against a concurrent getter or setter on + // free-threaded builds. Decref the old name only after the new one is + // stored and we've left the section: the slot then never points at a + // half-torn-down object, and an arbitrary finalizer (e.g. a str subclass + // __del__) won't run under the lock where it could reenter or deadlock. + PyObject *old; Py_BEGIN_CRITICAL_SECTION(op); - Py_XDECREF(func->func_name); + old = func->func_name; func->func_name = value; Py_END_CRITICAL_SECTION(); + Py_XDECREF(old); return 0; } From 2ab51a60cddbf9a60a7d53ded62c018b737839ad Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Mon, 15 Jun 2026 17:33:14 +0100 Subject: [PATCH 4/5] Fix another concurrency bug --- mypyc/lib-rt/function_wrapper.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/mypyc/lib-rt/function_wrapper.c b/mypyc/lib-rt/function_wrapper.c index 47495d50e2b82..0f2f03630b085 100644 --- a/mypyc/lib-rt/function_wrapper.c +++ b/mypyc/lib-rt/function_wrapper.c @@ -267,12 +267,13 @@ PyObject* CPyFunction_New(PyObject *module, const char *filename, const char *fu PyMutex_Lock(&type_init_mutex); if (!CPyFunctionType) { PyTypeObject *type = (PyTypeObject *)PyType_FromSpec(&CPyFunction_spec); + if (unlikely(!type)) { + PyMutex_Unlock(&type_init_mutex); + goto err; + } _Py_atomic_store_ptr_release(&CPyFunctionType, type); } PyMutex_Unlock(&type_init_mutex); - if (unlikely(!CPyFunctionType)) { - goto err; - } } #else if (!CPyFunctionType) { From fcbffb0409247a5a5045c4d6d96e73d490ded00d Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Tue, 16 Jun 2026 11:47:33 +0100 Subject: [PATCH 5/5] Simplify verbose comments --- mypyc/lib-rt/function_wrapper.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/mypyc/lib-rt/function_wrapper.c b/mypyc/lib-rt/function_wrapper.c index 0f2f03630b085..16b1b59303481 100644 --- a/mypyc/lib-rt/function_wrapper.c +++ b/mypyc/lib-rt/function_wrapper.c @@ -29,9 +29,7 @@ static void CPyFunction_dealloc(CPyFunction *m) { } static PyObject* CPyFunction_repr(CPyFunction *op) { - // Take a strong ref to the name via the getter, which guards against a - // concurrent CPyFunction_set_name freeing func_name from under us on - // free-threaded builds. It also handles lazy initialization of func_name. + // Use helper to get name for free threading safety. PyObject *name = CPyFunction_get_name((PyObject *)op, NULL); if (unlikely(name == NULL)) { return NULL; @@ -69,9 +67,6 @@ PyObject* CPyFunction_get_name(PyObject *op, void *context) { (void)context; CPyFunction *func = (CPyFunction *)op; PyObject *result; - // The critical section makes the lazy init and the incref atomic with - // respect to a concurrent CPyFunction_set_name on free-threaded builds, - // which would otherwise free func_name from under us. Py_BEGIN_CRITICAL_SECTION(op); if (unlikely(func->func_name == NULL)) { func->func_name = PyUnicode_InternFromString(((PyCFunctionObject *)func)->m_ml->ml_name); @@ -91,12 +86,7 @@ int CPyFunction_set_name(PyObject *op, PyObject *value, void *context) { } Py_INCREF(value); - // Store the new name and capture the old one under the critical section, - // which serializes the store against a concurrent getter or setter on - // free-threaded builds. Decref the old name only after the new one is - // stored and we've left the section: the slot then never points at a - // half-torn-down object, and an arbitrary finalizer (e.g. a str subclass - // __del__) won't run under the lock where it could reenter or deadlock. + // Decref outside critical section, since it could run arbitrary code. PyObject *old; Py_BEGIN_CRITICAL_SECTION(op); old = func->func_name; @@ -259,9 +249,7 @@ PyObject* CPyFunction_New(PyObject *module, const char *filename, const char *fu #ifdef Py_GIL_DISABLED // Double-checked locking: the common case (type already created) is a // lock-free atomic load. Only the first-time initialization takes the - // mutex, which serializes concurrent creators so we don't leak a type or - // race on the store. The release/acquire pairing ensures a thread that - // observes a non-NULL pointer also sees the fully initialized type. + // mutex, which serializes concurrent creators. if (!_Py_atomic_load_ptr_acquire(&CPyFunctionType)) { static PyMutex type_init_mutex = {0}; PyMutex_Lock(&type_init_mutex);