diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 50225623fe52db..50807e68e9a70c 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -767,6 +767,27 @@ _Py_TryIncref(PyObject *op) #endif } +// Enqueue an object to be freed possibly after some delay +#ifdef Py_GIL_DISABLED +PyAPI_FUNC(void) _PyObject_XDecRefDelayed(PyObject *obj); +#else +static inline void _PyObject_XDecRefDelayed(PyObject *obj) +{ + Py_XDECREF(obj); +} +#endif + +#ifdef Py_GIL_DISABLED +// Same as `Py_XSETREF` but in free-threading, it stores the object atomically +// and queues the old object to be decrefed at a safe point using QSBR. +PyAPI_FUNC(void) _PyObject_XSetRefDelayed(PyObject **p_obj, PyObject *obj); +#else +static inline void _PyObject_XSetRefDelayed(PyObject **p_obj, PyObject *obj) +{ + Py_XSETREF(*p_obj, obj); +} +#endif + #ifdef Py_REF_DEBUG extern void _PyInterpreterState_FinalizeRefTotal(PyInterpreterState *); extern void _Py_FinalizeRefTotal(_PyRuntimeState *); diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index 02537bdfef8598..e669a30b072d18 100644 --- a/Include/internal/pycore_pymem.h +++ b/Include/internal/pycore_pymem.h @@ -90,16 +90,6 @@ extern int _PyMem_DebugEnabled(void); // Enqueue a pointer to be freed possibly after some delay. extern void _PyMem_FreeDelayed(void *ptr); -// Enqueue an object to be freed possibly after some delay -#ifdef Py_GIL_DISABLED -PyAPI_FUNC(void) _PyObject_XDecRefDelayed(PyObject *obj); -#else -static inline void _PyObject_XDecRefDelayed(PyObject *obj) -{ - Py_XDECREF(obj); -} -#endif - // Periodically process delayed free requests. extern void _PyMem_ProcessDelayed(PyThreadState *tstate); diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py new file mode 100644 index 00000000000000..d01675eb38b370 --- /dev/null +++ b/Lib/test/test_free_threading/test_generators.py @@ -0,0 +1,51 @@ +import concurrent.futures +import unittest +from threading import Barrier +from unittest import TestCase +import random +import time + +from test.support import threading_helper, Py_GIL_DISABLED + +threading_helper.requires_working_threading(module=True) + + +def random_sleep(): + delay_us = random.randint(50, 100) + time.sleep(delay_us * 1e-6) + +def random_string(): + return ''.join(random.choice('0123456789ABCDEF') for _ in range(10)) + +def set_gen_name(g, b): + b.wait() + random_sleep() + g.__name__ = random_string() + return g.__name__ + +def set_gen_qualname(g, b): + b.wait() + random_sleep() + g.__qualname__ = random_string() + return g.__qualname__ + + +@unittest.skipUnless(Py_GIL_DISABLED, "Enable only in FT build") +class TestFTGenerators(TestCase): + NUM_THREADS = 4 + + def concurrent_write_with_func(self, func): + gen = (x for x in range(42)) + for j in range(1000): + with concurrent.futures.ThreadPoolExecutor(max_workers=self.NUM_THREADS) as executor: + b = Barrier(self.NUM_THREADS) + futures = {executor.submit(func, gen, b): i for i in range(self.NUM_THREADS)} + for fut in concurrent.futures.as_completed(futures): + gen_name = fut.result() + self.assertEqual(len(gen_name), 10) + + def test_concurrent_write(self): + with self.subTest(func=set_gen_name): + self.concurrent_write_with_func(func=set_gen_name) + with self.subTest(func=set_gen_qualname): + self.concurrent_write_with_func(func=set_gen_qualname) diff --git a/Objects/genobject.c b/Objects/genobject.c index da1462deaaa02c..d0cb75d2d17b07 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -704,7 +704,8 @@ static PyObject * gen_get_name(PyObject *self, void *Py_UNUSED(ignored)) { PyGenObject *op = _PyGen_CAST(self); - return Py_NewRef(op->gi_name); + PyObject *name = FT_ATOMIC_LOAD_PTR_ACQUIRE(op->gi_name); + return Py_NewRef(name); } static int @@ -718,7 +719,11 @@ gen_set_name(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) "__name__ must be set to a string object"); return -1; } - Py_XSETREF(op->gi_name, Py_NewRef(value)); + Py_BEGIN_CRITICAL_SECTION(self); + // gh-133931: To prevent use-after-free from other threads that reference + // the gi_name. + _PyObject_XSetRefDelayed(&op->gi_name, Py_NewRef(value)); + Py_END_CRITICAL_SECTION(); return 0; } @@ -726,7 +731,8 @@ static PyObject * gen_get_qualname(PyObject *self, void *Py_UNUSED(ignored)) { PyGenObject *op = _PyGen_CAST(self); - return Py_NewRef(op->gi_qualname); + PyObject *qualname = FT_ATOMIC_LOAD_PTR_ACQUIRE(op->gi_qualname); + return Py_NewRef(qualname); } static int @@ -740,7 +746,11 @@ gen_set_qualname(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) "__qualname__ must be set to a string object"); return -1; } - Py_XSETREF(op->gi_qualname, Py_NewRef(value)); + Py_BEGIN_CRITICAL_SECTION(self); + // gh-133931: To prevent use-after-free from other threads that reference + // the gi_qualname. + _PyObject_XSetRefDelayed(&op->gi_qualname, Py_NewRef(value)); + Py_END_CRITICAL_SECTION(); return 0; } diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index d3931aab623b70..d4b8327cb73b37 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1231,6 +1231,21 @@ _PyObject_XDecRefDelayed(PyObject *ptr) } #endif +#ifdef Py_GIL_DISABLED +void +_PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) +{ + PyObject *old = *ptr; + FT_ATOMIC_STORE_PTR_RELEASE(*ptr, value); + if (old == NULL) { + return; + } + if (!_Py_IsImmortal(old)) { + _PyObject_XDecRefDelayed(old); + } +} +#endif + static struct _mem_work_chunk * work_queue_first(struct llist_node *head) { diff --git a/Objects/typeobject.c b/Objects/typeobject.c index db923c164774b7..b9d549610693c1 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3967,13 +3967,9 @@ _PyObject_SetDict(PyObject *obj, PyObject *value) return -1; } Py_BEGIN_CRITICAL_SECTION(obj); - PyObject *olddict = *dictptr; - FT_ATOMIC_STORE_PTR_RELEASE(*dictptr, Py_NewRef(value)); -#ifdef Py_GIL_DISABLED - _PyObject_XDecRefDelayed(olddict); -#else - Py_XDECREF(olddict); -#endif + // gh-133980: To prevent use-after-free from other threads that reference + // the __dict__ + _PyObject_XSetRefDelayed(dictptr, Py_NewRef(value)); Py_END_CRITICAL_SECTION(); return 0; }