Skip to content

Commit 2e332b2

Browse files
[3.14] gh-149816: Fix race in subtype_getweakref in free-threading build (gh-150247) (gh-159363)
(cherry picked from commit f674db1) Co-authored-by: Abhishek Tiwari <Abhi210@users.noreply.github.com>
1 parent 989b254 commit 2e332b2

3 files changed

Lines changed: 110 additions & 4 deletions

File tree

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
"""Regression tests for weakref races in subtype_getweakref() (gh-149816)."""
2+
3+
import gc
4+
import sys
5+
import unittest
6+
import weakref
7+
8+
from test.support import threading_helper
9+
10+
11+
@threading_helper.requires_working_threading()
12+
class TestWeakrefRaces(unittest.TestCase):
13+
14+
ITERATIONS = 5_000
15+
16+
def test_getweakref_no_crash(self):
17+
"""obj.__weakref__ racing with concurrent weakref creation/destruction."""
18+
class Target:
19+
pass
20+
21+
obj = Target()
22+
23+
def reader():
24+
for _ in range(self.ITERATIONS):
25+
ref = obj.__weakref__
26+
self.assertIn(type(ref), (type(None), weakref.ref))
27+
if ref is not None:
28+
self.assertIn(ref(), (None, obj))
29+
30+
def mutator():
31+
for _ in range(self.ITERATIONS):
32+
weakref.ref(obj, lambda _: None)
33+
34+
threading_helper.run_concurrently([reader, mutator])
35+
36+
def test_getweakref_return_type(self):
37+
"""__weakref__ must return None or a live weakref.ref."""
38+
class Target:
39+
pass
40+
41+
obj = Target()
42+
43+
self.assertIsNone(obj.__weakref__)
44+
45+
ref = weakref.ref(obj)
46+
result = obj.__weakref__
47+
self.assertIsInstance(result, weakref.ref)
48+
self.assertIs(result(), obj)
49+
50+
del ref, result
51+
gc.collect()
52+
self.assertIsNone(obj.__weakref__)
53+
54+
def test_getweakref_no_refcount_leak(self):
55+
"""Each __weakref__ access must not inflate the weakref's refcount."""
56+
class Target:
57+
pass
58+
59+
obj = Target()
60+
ref = weakref.ref(obj)
61+
before = sys.getrefcount(ref)
62+
63+
for _ in range(1_000):
64+
r = obj.__weakref__
65+
del r
66+
67+
gc.collect()
68+
self.assertEqual(sys.getrefcount(ref), before)
69+
70+
def test_getweakref_many_readers(self):
71+
"""Multiple concurrent readers and mutators must not crash."""
72+
class Target:
73+
pass
74+
75+
obj = Target()
76+
n = 4
77+
iters = self.ITERATIONS // n
78+
79+
def reader():
80+
for _ in range(iters):
81+
ref = obj.__weakref__
82+
self.assertIn(type(ref), (type(None), weakref.ref))
83+
84+
def mutator():
85+
for _ in range(iters):
86+
weakref.ref(obj, lambda _: None)
87+
88+
threading_helper.run_concurrently([reader] * n + [mutator] * n)
89+
90+
91+
if __name__ == "__main__":
92+
unittest.main()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a Use-After-Free race condition in ``subtype_getweakref()`` during
2+
concurrent weakref creation and destruction in the free-threading build.

‎Objects/typeobject.c‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3832,12 +3832,24 @@ subtype_getweakref(PyObject *obj, void *context)
38323832
_PyObject_ASSERT((PyObject *)type,
38333833
((type->tp_weaklistoffset + (Py_ssize_t)sizeof(PyObject *))
38343834
<= type->tp_basicsize));
3835+
3836+
/* Synchronize with weakref creation/destruction so the list head cannot
3837+
* be removed and freed between loading it and taking a reference. */
3838+
LOCK_WEAKREFS(obj);
38353839
weaklistptr = (PyObject **)((char *)obj + type->tp_weaklistoffset);
3836-
if (*weaklistptr == NULL)
3837-
result = Py_None;
3838-
else
3840+
if (*weaklistptr == NULL) {
3841+
result = Py_NewRef(Py_None);
3842+
}
3843+
else if (_Py_TryIncref(*weaklistptr)) {
38393844
result = *weaklistptr;
3840-
return Py_NewRef(result);
3845+
}
3846+
else {
3847+
result = Py_NewRef(Py_None);
3848+
}
3849+
3850+
UNLOCK_WEAKREFS(obj);
3851+
3852+
return result;
38413853
}
38423854

38433855
/* Three variants on the subtype_getsets list. */

0 commit comments

Comments
 (0)