From 09baa0441c5d9a3bba76c1c020225f221d743260 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 20 May 2025 12:06:05 -0400 Subject: [PATCH 01/14] gh-133931: Introduce _PyObject_XSetRefDelayed to replace Py_XSETREF --- Include/internal/pycore_pymem.h | 9 +++++++++ Objects/genobject.c | 9 +++++++-- Objects/obmalloc.c | 11 +++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index 02537bdfef8598..6d6608f0c70618 100644 --- a/Include/internal/pycore_pymem.h +++ b/Include/internal/pycore_pymem.h @@ -100,6 +100,15 @@ static inline void _PyObject_XDecRefDelayed(PyObject *obj) } #endif +#ifdef Py_GIL_DISABLED +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 + // Periodically process delayed free requests. extern void _PyMem_ProcessDelayed(PyThreadState *tstate); diff --git a/Objects/genobject.c b/Objects/genobject.c index da1462deaaa02c..9ecf6c6863240a 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -10,6 +10,7 @@ #include "pycore_gc.h" // _PyGC_CLEAR_FINALIZED() #include "pycore_genobject.h" // _PyGen_SetStopIterationValue() #include "pycore_interpframe.h" // _PyFrame_GetCode() +#include "pycore_pymem.h" // _PyObject_XSetRefDelayed() #include "pycore_modsupport.h" // _PyArg_CheckPositional() #include "pycore_object.h" // _PyObject_GC_UNTRACK() #include "pycore_opcode_utils.h" // RESUME_AFTER_YIELD_FROM @@ -718,7 +719,9 @@ 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); + _PyObject_XSetRefDelayed(&op->gi_name, value); + Py_END_CRITICAL_SECTION(); return 0; } @@ -740,7 +743,9 @@ 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); + _PyObject_XSetRefDelayed(&op->gi_qualname, value); + Py_END_CRITICAL_SECTION(); return 0; } diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index d3931aab623b70..fc797934cb7d92 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1231,6 +1231,17 @@ _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, Py_NewRef(value)); + _PyObject_XDecRefDelayed(old); + Py_XDECREF(old); +} +#endif + static struct _mem_work_chunk * work_queue_first(struct llist_node *head) { From 1ee08256853590e2cea5aace7898fccdc12d4f1f Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 20 May 2025 14:25:48 -0400 Subject: [PATCH 02/14] fix --- Objects/obmalloc.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index fc797934cb7d92..cee3c9c4b6776a 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1238,7 +1238,6 @@ _PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) PyObject *old = *ptr; FT_ATOMIC_STORE_PTR_RELEASE(*ptr, Py_NewRef(value)); _PyObject_XDecRefDelayed(old); - Py_XDECREF(old); } #endif From 753df7a27ef5a02f44670b8ef88718d227945d49 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 20 May 2025 21:36:09 -0400 Subject: [PATCH 03/14] Address code review --- Objects/genobject.c | 4 ++-- Objects/obmalloc.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Objects/genobject.c b/Objects/genobject.c index 9ecf6c6863240a..c46faf3a94d4eb 100644 --- a/Objects/genobject.c +++ b/Objects/genobject.c @@ -720,7 +720,7 @@ gen_set_name(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) return -1; } Py_BEGIN_CRITICAL_SECTION(self); - _PyObject_XSetRefDelayed(&op->gi_name, value); + _PyObject_XSetRefDelayed(&op->gi_name, Py_NewRef(value)); Py_END_CRITICAL_SECTION(); return 0; } @@ -744,7 +744,7 @@ gen_set_qualname(PyObject *self, PyObject *value, void *Py_UNUSED(ignored)) return -1; } Py_BEGIN_CRITICAL_SECTION(self); - _PyObject_XSetRefDelayed(&op->gi_qualname, value); + _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 cee3c9c4b6776a..48e8028fce6453 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1236,7 +1236,7 @@ void _PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) { PyObject *old = *ptr; - FT_ATOMIC_STORE_PTR_RELEASE(*ptr, Py_NewRef(value)); + FT_ATOMIC_STORE_PTR_RELEASE(*ptr, value); _PyObject_XDecRefDelayed(old); } #endif From a168de02d7e22c1ab2c5e6eb78891470ab97c77f Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 21 May 2025 00:48:01 -0400 Subject: [PATCH 04/14] fix --- Objects/obmalloc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 48e8028fce6453..b445bbbad083fc 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1237,7 +1237,12 @@ _PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) { PyObject *old = *ptr; FT_ATOMIC_STORE_PTR_RELEASE(*ptr, value); - _PyObject_XDecRefDelayed(old); + if (!_Py_IsImmortal(old)) { + _PyObject_XDecRefDelayed(old); + } + else { + Py_XDECREF(old); + } } #endif From e7f6d01cdcdb17ab57cdf33109464ae748d64696 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 7 Jun 2025 22:00:45 +0900 Subject: [PATCH 05/14] fix issue --- Objects/obmalloc.c | 3 +++ Objects/typeobject.c | 8 +------- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index b445bbbad083fc..86b36cb0665aa1 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1237,6 +1237,9 @@ _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); } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index db923c164774b7..6763cbe21c1d4d 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3967,13 +3967,7 @@ _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 + _PyObject_XSetRefDelayed(dictptr, Py_NewRef(value)); Py_END_CRITICAL_SECTION(); return 0; } From 628732235edf858bc4d80e6522d3d43baccc8ff7 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 7 Jun 2025 22:15:26 +0900 Subject: [PATCH 06/14] nit --- Objects/obmalloc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 86b36cb0665aa1..53ffd5e3e1cea9 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1237,14 +1237,14 @@ _PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) { PyObject *old = *ptr; FT_ATOMIC_STORE_PTR_RELEASE(*ptr, value); - if (old == NULL) { + if (old == _Py_NULL) { return; } if (!_Py_IsImmortal(old)) { _PyObject_XDecRefDelayed(old); } else { - Py_XDECREF(old); + Py_DECREF(old); } } #endif From 12a701657565b44d44a4b19f6449b2ffb4ef65c5 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Sat, 7 Jun 2025 22:33:27 +0900 Subject: [PATCH 07/14] Add comment --- Include/internal/pycore_pymem.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index 6d6608f0c70618..4c9eda41f3ccdf 100644 --- a/Include/internal/pycore_pymem.h +++ b/Include/internal/pycore_pymem.h @@ -101,6 +101,9 @@ static inline void _PyObject_XDecRefDelayed(PyObject *obj) #endif #ifdef Py_GIL_DISABLED +// This is the delayed-free equivalent of Py_XSETREF(), providing the same +// atomic reference assignment semantics but with deferred cleanup suitable +// for concurrent access patterns in free-threaded Python. PyAPI_FUNC(void) _PyObject_XSetRefDelayed(PyObject **p_obj, PyObject *obj); #else static inline void _PyObject_XSetRefDelayed(PyObject **p_obj, PyObject *obj) From 5f42224849af4b24ed3042bd0dd2409f84568592 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 10 Jun 2025 20:17:56 +0900 Subject: [PATCH 08/14] Update Objects/obmalloc.c Co-authored-by: Kumar Aditya --- Objects/obmalloc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 53ffd5e3e1cea9..9011f79d0256aa 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1237,7 +1237,7 @@ _PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) { PyObject *old = *ptr; FT_ATOMIC_STORE_PTR_RELEASE(*ptr, value); - if (old == _Py_NULL) { + if (old == NULL) { return; } if (!_Py_IsImmortal(old)) { From 388f0ca7e5078458dd6c2b724619c429ff94b7dd Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 10 Jun 2025 21:06:26 +0900 Subject: [PATCH 09/14] Update Include/internal/pycore_pymem.h Co-authored-by: Kumar Aditya --- Include/internal/pycore_pymem.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index 4c9eda41f3ccdf..77785e6d356f18 100644 --- a/Include/internal/pycore_pymem.h +++ b/Include/internal/pycore_pymem.h @@ -101,9 +101,8 @@ static inline void _PyObject_XDecRefDelayed(PyObject *obj) #endif #ifdef Py_GIL_DISABLED -// This is the delayed-free equivalent of Py_XSETREF(), providing the same -// atomic reference assignment semantics but with deferred cleanup suitable -// for concurrent access patterns in free-threaded Python. +// 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) From 822036767e491c046d53d1d1daa57726f71c189f Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 10 Jun 2025 22:05:28 +0900 Subject: [PATCH 10/14] Update Include/internal/pycore_pymem.h Co-authored-by: Victor Stinner --- Include/internal/pycore_pymem.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_pymem.h b/Include/internal/pycore_pymem.h index 77785e6d356f18..8bbc349278594e 100644 --- a/Include/internal/pycore_pymem.h +++ b/Include/internal/pycore_pymem.h @@ -102,7 +102,7 @@ static inline void _PyObject_XDecRefDelayed(PyObject *obj) #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 +// 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) From c82e0fb920a3338edf0238fae2460e8601b11907 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Tue, 10 Jun 2025 22:12:22 +0900 Subject: [PATCH 11/14] Address code review --- Objects/obmalloc.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index 9011f79d0256aa..d4b8327cb73b37 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1243,9 +1243,6 @@ _PyObject_XSetRefDelayed(PyObject **ptr, PyObject *value) if (!_Py_IsImmortal(old)) { _PyObject_XDecRefDelayed(old); } - else { - Py_DECREF(old); - } } #endif From f99c8f1890112711d0dd9fe6ef28044903cf104d Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 11 Jun 2025 12:24:48 +0900 Subject: [PATCH 12/14] Add test --- .../test_free_threading/test_generators.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 Lib/test/test_free_threading/test_generators.py 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..32581aa59993fa --- /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) From 5dc24a35a3c4e0e66e4a819a26bda135e9228091 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 11 Jun 2025 12:40:51 +0900 Subject: [PATCH 13/14] fix --- Lib/test/test_free_threading/test_generators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py index 32581aa59993fa..b0e80199c33301 100644 --- a/Lib/test/test_free_threading/test_generators.py +++ b/Lib/test/test_free_threading/test_generators.py @@ -33,7 +33,7 @@ def set_gen_qualname(g, b): @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): From 99d2718a0284cd1f66c19c8bce8024167ca07eb4 Mon Sep 17 00:00:00 2001 From: Donghee Na Date: Wed, 11 Jun 2025 12:43:11 +0900 Subject: [PATCH 14/14] fix --- Lib/test/test_free_threading/test_generators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_free_threading/test_generators.py b/Lib/test/test_free_threading/test_generators.py index b0e80199c33301..d01675eb38b370 100644 --- a/Lib/test/test_free_threading/test_generators.py +++ b/Lib/test/test_free_threading/test_generators.py @@ -33,7 +33,7 @@ def set_gen_qualname(g, b): @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):