diff --git a/Lib/test/test_free_threading/test_weakref.py b/Lib/test/test_free_threading/test_weakref.py new file mode 100644 index 000000000000000..7d047244828f6b9 --- /dev/null +++ b/Lib/test/test_free_threading/test_weakref.py @@ -0,0 +1,92 @@ +"""Regression tests for weakref races in subtype_getweakref() (gh-149816).""" + +import gc +import sys +import unittest +import weakref + +from test.support import threading_helper + + +@threading_helper.requires_working_threading() +class TestWeakrefRaces(unittest.TestCase): + + ITERATIONS = 5_000 + + def test_getweakref_no_crash(self): + """obj.__weakref__ racing with concurrent weakref creation/destruction.""" + class Target: + pass + + obj = Target() + + def reader(): + for _ in range(self.ITERATIONS): + ref = obj.__weakref__ + self.assertIn(type(ref), (type(None), weakref.ref)) + if ref is not None: + self.assertIn(ref(), (None, obj)) + + def mutator(): + for _ in range(self.ITERATIONS): + weakref.ref(obj, lambda _: None) + + threading_helper.run_concurrently([reader, mutator]) + + def test_getweakref_return_type(self): + """__weakref__ must return None or a live weakref.ref.""" + class Target: + pass + + obj = Target() + + self.assertIsNone(obj.__weakref__) + + ref = weakref.ref(obj) + result = obj.__weakref__ + self.assertIsInstance(result, weakref.ref) + self.assertIs(result(), obj) + + del ref, result + gc.collect() + self.assertIsNone(obj.__weakref__) + + def test_getweakref_no_refcount_leak(self): + """Each __weakref__ access must not inflate the weakref's refcount.""" + class Target: + pass + + obj = Target() + ref = weakref.ref(obj) + before = sys.getrefcount(ref) + + for _ in range(1_000): + r = obj.__weakref__ + del r + + gc.collect() + self.assertEqual(sys.getrefcount(ref), before) + + def test_getweakref_many_readers(self): + """Multiple concurrent readers and mutators must not crash.""" + class Target: + pass + + obj = Target() + n = 4 + iters = self.ITERATIONS // n + + def reader(): + for _ in range(iters): + ref = obj.__weakref__ + self.assertIn(type(ref), (type(None), weakref.ref)) + + def mutator(): + for _ in range(iters): + weakref.ref(obj, lambda _: None) + + threading_helper.run_concurrently([reader] * n + [mutator] * n) + + +if __name__ == "__main__": + unittest.main() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-05-22-17-10-29.gh-issue-149816.u1-iqT.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-05-22-17-10-29.gh-issue-149816.u1-iqT.rst new file mode 100644 index 000000000000000..0faa5f7b39a415c --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-05-22-17-10-29.gh-issue-149816.u1-iqT.rst @@ -0,0 +1,2 @@ +Fix a Use-After-Free race condition in ``subtype_getweakref()`` during +concurrent weakref creation and destruction in the free-threading build. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b77f3e3bce54551..e26597781049259 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4075,12 +4075,24 @@ subtype_getweakref(PyObject *obj, void *context) _PyObject_ASSERT((PyObject *)type, ((type->tp_weaklistoffset + (Py_ssize_t)sizeof(PyObject *)) <= type->tp_basicsize)); + + /* Synchronize with weakref creation/destruction so the list head cannot + * be removed and freed between loading it and taking a reference. */ + LOCK_WEAKREFS(obj); weaklistptr = (PyObject **)((char *)obj + type->tp_weaklistoffset); - if (*weaklistptr == NULL) - result = Py_None; - else + if (*weaklistptr == NULL) { + result = Py_NewRef(Py_None); + } + else if (_Py_TryIncref(*weaklistptr)) { result = *weaklistptr; - return Py_NewRef(result); + } + else { + result = Py_NewRef(Py_None); + } + + UNLOCK_WEAKREFS(obj); + + return result; } /* getset definitions for common descriptors */