Skip to content

Commit 00c5450

Browse files
authored
gh-159564: Make weakref dereference lock-free. (#159578)
1 parent 763fb33 commit 00c5450

4 files changed

Lines changed: 46 additions & 12 deletions

File tree

‎Include/internal/pycore_weakref.h‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,26 +74,27 @@ static inline PyObject* _PyWeakref_GET_REF(PyObject *ref_obj)
7474
assert(PyWeakref_Check(ref_obj));
7575
PyWeakReference *ref = _Py_CAST(PyWeakReference*, ref_obj);
7676

77-
PyObject *obj = FT_ATOMIC_LOAD_PTR(ref->wr_object);
77+
#ifdef Py_GIL_DISABLED
78+
PyObject *obj = _Py_atomic_load_ptr(&ref->wr_object);
7879
if (obj == Py_None) {
7980
// clear_weakref() was called
8081
return NULL;
8182
}
82-
83-
LOCK_WEAKREFS(obj);
84-
#ifdef Py_GIL_DISABLED
85-
if (ref->wr_object == Py_None) {
83+
if (_Py_TryIncrefCompare(&ref->wr_object, obj)) {
84+
return obj;
85+
}
86+
return NULL;
87+
#else
88+
PyObject *obj = ref->wr_object;
89+
if (obj == Py_None) {
8690
// clear_weakref() was called
87-
UNLOCK_WEAKREFS(obj);
8891
return NULL;
8992
}
90-
#endif
9193
if (_Py_TryIncref(obj)) {
92-
UNLOCK_WEAKREFS(obj);
9394
return obj;
9495
}
95-
UNLOCK_WEAKREFS(obj);
9696
return NULL;
97+
#endif
9798
}
9899

99100
static inline int _PyWeakref_IS_DEAD(PyObject *ref_obj)
@@ -108,8 +109,8 @@ static inline int _PyWeakref_IS_DEAD(PyObject *ref_obj)
108109
}
109110
else {
110111
LOCK_WEAKREFS(obj);
111-
// See _PyWeakref_GET_REF() for the rationale of this test
112112
#ifdef Py_GIL_DISABLED
113+
// The weakref may have been cleared before the lock was acquired.
113114
ret = (ref->wr_object == Py_None) || _is_dead(obj);
114115
#else
115116
ret = _is_dead(obj);

‎Lib/test/test_weakref.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1026,6 +1026,36 @@ def cb(wparent):
10261026
del root
10271027
gc.collect()
10281028

1029+
@support.skip_if_sanitizer("requires deep stack", thread=True)
1030+
@support.skip_emscripten_stack_overflow()
1031+
@support.skip_wasi_stack_overflow()
1032+
def test_trashcan_deferred_dealloc(self):
1033+
# When the C stack is nearly exhausted, the trashcan defers
1034+
# deallocation of an object whose refcount has dropped to 0, leaving
1035+
# its weakrefs uncleared. Calling such a weakref must return None
1036+
# rather than resurrecting the dead object.
1037+
class Node(list):
1038+
pass
1039+
1040+
found = []
1041+
class Trigger:
1042+
def __init__(self, wr):
1043+
self.wr = wr
1044+
def __del__(self):
1045+
if self.wr() is not None:
1046+
found.append(True)
1047+
1048+
head = cur = Node()
1049+
for i in range(200_000):
1050+
child = Node()
1051+
# list_dealloc() clears items last to first: the child is
1052+
# deallocated (or deferred) before the trigger runs.
1053+
cur.extend([Trigger(weakref.ref(child)), child])
1054+
cur = child
1055+
del cur, child
1056+
del head
1057+
self.assertEqual(found, [])
1058+
10291059
def test_callback_attribute(self):
10301060
x = Object(1)
10311061
callback = lambda ref: None
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Improve the performance and thread scaling of dereferencing weak references
2+
in the :term:`free-threaded build`. Dereferencing a weak reference no longer
3+
takes a lock.

‎Objects/weakrefobject.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,9 @@ clear_weakref_lock_held(PyWeakReference *self, PyObject **callback)
9292
/* If 'self' is the end of the list (and thus self->wr_next ==
9393
NULL) then the weakref list itself (and thus the value of *list)
9494
will end up being set to NULL. */
95-
FT_ATOMIC_STORE_PTR(*list, self->wr_next);
95+
FT_ATOMIC_STORE_PTR_RELEASE(*list, self->wr_next);
9696
}
97-
FT_ATOMIC_STORE_PTR(self->wr_object, Py_None);
97+
FT_ATOMIC_STORE_PTR_RELEASE(self->wr_object, Py_None);
9898
if (self->wr_prev != NULL) {
9999
self->wr_prev->wr_next = self->wr_next;
100100
}

0 commit comments

Comments
 (0)