Skip to content

Commit f674db1

Browse files
authored
gh-149816: Fix race in subtype_getweakref in free-threading build (gh-150247)
1 parent 9c15cc9 commit f674db1

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
@@ -4058,12 +4058,24 @@ subtype_getweakref(PyObject *obj, void *context)
40584058
_PyObject_ASSERT((PyObject *)type,
40594059
((type->tp_weaklistoffset + (Py_ssize_t)sizeof(PyObject *))
40604060
<= type->tp_basicsize));
4061+
4062+
/* Synchronize with weakref creation/destruction so the list head cannot
4063+
* be removed and freed between loading it and taking a reference. */
4064+
LOCK_WEAKREFS(obj);
40614065
weaklistptr = (PyObject **)((char *)obj + type->tp_weaklistoffset);
4062-
if (*weaklistptr == NULL)
4063-
result = Py_None;
4064-
else
4066+
if (*weaklistptr == NULL) {
4067+
result = Py_NewRef(Py_None);
4068+
}
4069+
else if (_Py_TryIncref(*weaklistptr)) {
40654070
result = *weaklistptr;
4066-
return Py_NewRef(result);
4071+
}
4072+
else {
4073+
result = Py_NewRef(Py_None);
4074+
}
4075+
4076+
UNLOCK_WEAKREFS(obj);
4077+
4078+
return result;
40674079
}
40684080

40694081
/* getset definitions for common descriptors */

0 commit comments

Comments
 (0)