From b6378e010fad82b9128c7dddcb86da347a15f2f1 Mon Sep 17 00:00:00 2001 From: David Klement Date: Thu, 5 Mar 2026 15:14:22 +0100 Subject: [PATCH 1/5] Immutability: Use ob_refcnt instead of ob_refcnt_full --- Python/immutability.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Python/immutability.c b/Python/immutability.c index 5cf62319d9f5f8..2c99bc0bf295a1 100644 --- a/Python/immutability.c +++ b/Python/immutability.c @@ -1725,10 +1725,7 @@ int _Py_DecRef_Immutable(PyObject *op) #if SIZEOF_VOID_P > 4 - Py_ssize_t old = _Py_atomic_add_int64(&op->ob_refcnt_full, -1); - // The ssize_t might be too big, so mask to 32 bits as that is the size of - // ob_refcnt. - old = old & 0xFFFFFFFF; + uint32_t old = _Py_atomic_add_uint32(&op->ob_refcnt, -1); #else // TODO(Immutable 32): Find SCC if required. @@ -1788,7 +1785,7 @@ void _Py_RefcntAdd_Immutable(PyObject *op, Py_ssize_t increment) // Increment the reference count of an immutable object. assert(_Py_IsImmutable(op)); #if SIZEOF_VOID_P > 4 - _Py_atomic_add_int64(&op->ob_refcnt_full, increment); + _Py_atomic_add_uint32(&op->ob_refcnt, increment); #else _Py_atomic_add_ssize(&op->ob_refcnt, increment); #endif From 24a9eb3469756eeeb49ea296603316a78bf25178 Mon Sep 17 00:00:00 2001 From: David Klement Date: Thu, 5 Mar 2026 16:40:22 +0100 Subject: [PATCH 2/5] Immutability: Make weakref operations thread-safe --- Include/internal/pycore_object.h | 4 ++ Include/internal/pycore_weakref.h | 68 ++++++++++++++++++++---------- Modules/_weakref.c | 4 +- Objects/weakrefobject.c | 70 ++++++++++++++++++++++--------- Python/immutability.c | 42 +++++++++++++++++++ 5 files changed, 146 insertions(+), 42 deletions(-) diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index 20012948234d13..0a0061e4c9b29c 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -813,6 +813,7 @@ _Py_TryIncref(PyObject *op) #ifdef Py_GIL_DISABLED return _Py_TryIncrefFast(op) || _Py_TryIncRefShared(op); #else + assert(!_Py_IsImmutable(op) && "Use _Py_TryIncref_Immutable for immutable objects"); if (Py_REFCNT(op) > 0) { Py_INCREF(op); return 1; @@ -821,6 +822,9 @@ _Py_TryIncref(PyObject *op) #endif } +int _Py_TryIncref_Immutable(PyObject *op); +int _Py_IsDead_Immutable(PyObject *op); + // Enqueue an object to be freed possibly after some delay #ifdef Py_GIL_DISABLED PyAPI_FUNC(void) _PyObject_XDecRefDelayed(PyObject *obj); diff --git a/Include/internal/pycore_weakref.h b/Include/internal/pycore_weakref.h index 4ed8928c0b92a8..64854090474781 100644 --- a/Include/internal/pycore_weakref.h +++ b/Include/internal/pycore_weakref.h @@ -37,11 +37,14 @@ extern "C" { #else -#define LOCK_WEAKREFS(obj) -#define UNLOCK_WEAKREFS(obj) +// Lock used for weakrefs to immutable objects +extern PyMutex _PyWeakref_Lock; -#define LOCK_WEAKREFS_FOR_WR(wr) -#define UNLOCK_WEAKREFS_FOR_WR(wr) +#define LOCK_WEAKREFS(obj) PyMutex_LockFlags(&_PyWeakref_Lock, _Py_LOCK_DONT_DETACH) +#define UNLOCK_WEAKREFS(obj) PyMutex_Unlock(&_PyWeakref_Lock) + +#define LOCK_WEAKREFS_FOR_WR(wr) PyMutex_LockFlags(&_PyWeakref_Lock, _Py_LOCK_DONT_DETACH) +#define UNLOCK_WEAKREFS_FOR_WR(wr) PyMutex_Unlock(&_PyWeakref_Lock) #define FT_CLEAR_WEAKREFS(obj, weakref_list) \ do { \ @@ -65,35 +68,59 @@ static inline int _is_dead(PyObject *obj) Py_ssize_t shared = _Py_atomic_load_ssize_relaxed(&obj->ob_ref_shared); return shared == _Py_REF_SHARED(0, _Py_REF_MERGED); #else - return (Py_REFCNT(obj) == 0); + if (_Py_IsImmutable(obj)) { + return _Py_IsDead_Immutable(obj); + } + else { + return (Py_REFCNT(obj) == 0); + } #endif } +static inline PyObject* get_ref_lock_held(PyWeakReference *ref, PyObject *obj) +{ +#ifdef Py_GIL_DISABLED + // Need to check again because the object could have been deallocated + if (ref->wr_object == Py_None) { + // clear_weakref() was called + return NULL; + } + if (_Py_TryIncref(obj)) { + return obj; + } +#else + if (_Py_IsImmutable(obj)) { + // Need to check again because the object could have been deallocated + if (ref->wr_object == Py_None) { + // clear_weakref() was called + return NULL; + } + if (_Py_TryIncref_Immutable(obj)) { + return obj; + } + } + else if (_Py_TryIncref(obj)) { + return obj; + } +#endif + return NULL; +} + static inline PyObject* _PyWeakref_GET_REF(PyObject *ref_obj) { assert(PyWeakref_Check(ref_obj)); PyWeakReference *ref = _Py_CAST(PyWeakReference*, ref_obj); - PyObject *obj = FT_ATOMIC_LOAD_PTR(ref->wr_object); + PyObject *obj = _Py_atomic_load_ptr(&ref->wr_object); if (obj == Py_None) { // clear_weakref() was called return NULL; } LOCK_WEAKREFS(obj); -#ifdef Py_GIL_DISABLED - if (ref->wr_object == Py_None) { - // clear_weakref() was called - UNLOCK_WEAKREFS(obj); - return NULL; - } -#endif - if (_Py_TryIncref(obj)) { - UNLOCK_WEAKREFS(obj); - return obj; - } + PyObject* result = get_ref_lock_held(ref, obj); UNLOCK_WEAKREFS(obj); - return NULL; + return result; } static inline int _PyWeakref_IS_DEAD(PyObject *ref_obj) @@ -108,12 +135,9 @@ static inline int _PyWeakref_IS_DEAD(PyObject *ref_obj) } else { LOCK_WEAKREFS(obj); + // Immutable objects and free-threaded builds require a new check // See _PyWeakref_GET_REF() for the rationale of this test -#ifdef Py_GIL_DISABLED ret = (ref->wr_object == Py_None) || _is_dead(obj); -#else - ret = _is_dead(obj); -#endif UNLOCK_WEAKREFS(obj); } return ret; diff --git a/Modules/_weakref.c b/Modules/_weakref.c index ecaa08ff60f203..ec5c43b080378f 100644 --- a/Modules/_weakref.c +++ b/Modules/_weakref.c @@ -88,7 +88,9 @@ _weakref_getweakrefs(PyObject *module, PyObject *object) PyWeakReference *current = *GET_WEAKREFS_LISTPTR(object); while (current != NULL) { PyObject *curobj = (PyObject *) current; - if (_Py_TryIncref(curobj)) { + int incref_res = _Py_IsImmutable(curobj) ? + _Py_TryIncref_Immutable(curobj) : _Py_TryIncref(curobj); + if (incref_res) { if (PyList_Append(result, curobj)) { UNLOCK_WEAKREFS(object); Py_DECREF(curobj); diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index f6af75951ba0b2..9db7c7eab1f1a0 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -20,7 +20,7 @@ * * For now we've chosen to address this in a straightforward way: * - * - The weakref's hash is protected using the weakref's per-object lock. + * - The weakref's hash is protected using atomic operations. * - The other mutable is protected by a striped lock keyed on the referenced * object's address. * - The striped lock must be locked using `_Py_LOCK_DONT_DETACH` in order to @@ -32,8 +32,40 @@ * without acquiring any locks. */ +#else +/* + * Thread-safety for immutable objects + * =================================== + * + * Immutable objects, and their weakref lists, are shared across interpreters. + * Moreover, basic weakrefs pointing to immutable objects are shared. + * We need to protect mutable state of: + * + * - The weakref (wr_object, hash, wr_callback) + * - The referenced object (its head-of-list pointer) + * - The linked list of weakrefs + * + * For now we've chosen to address this in the following way: + * + * - The weakref's hash is protected using atomic operations. + * - The other mutable state is protected by a global lock. + * - The lock must be locked using `_Py_LOCK_DONT_DETACH` in order to + * support atomic deletion from WeakValueDictionaries. As a result, we must + * be careful not to perform any operations that could suspend while the + * lock is held. + * + * We also need to handle refcounts for the weakref object and the callback. + * TODO(Immutable-weakrefs): Implement this; handle weakrefs with callbacks. + * + * - Basic weakrefs pointing to immutable objects are marked as immutable, + * which turns on atomic reference counting. + * + * Immutable objects are never GC-collected. + */ #endif +PyMutex _PyWeakref_Lock; + #define GET_WEAKREFS_LISTPTR(o) \ ((PyWeakReference **) _PyObject_GET_WEAKREFS_LISTPTR(o)) @@ -84,9 +116,9 @@ clear_weakref_lock_held(PyWeakReference *self, PyObject **callback) /* If 'self' is the end of the list (and thus self->wr_next == NULL) then the weakref list itself (and thus the value of *list) will end up being set to NULL. */ - FT_ATOMIC_STORE_PTR(*list, self->wr_next); + _Py_atomic_store_ptr(list, self->wr_next); } - FT_ATOMIC_STORE_PTR(self->wr_object, Py_None); + _Py_atomic_store_ptr(&self->wr_object, Py_None); if (self->wr_prev != NULL) { self->wr_prev->wr_next = self->wr_next; } @@ -194,28 +226,22 @@ weakref_vectorcall(PyObject *self, PyObject *const *args, } static Py_hash_t -weakref_hash_lock_held(PyWeakReference *self) +weakref_hash(PyObject *op) { - if (self->hash != -1) - return self->hash; + // Immutable objects and free-threaded builds require atomic operations + PyWeakReference *self = _PyWeakref_CAST(op); + Py_hash_t hash = _Py_atomic_load_ssize_relaxed(&self->hash); + if (hash != -1) { + return hash; + } PyObject* obj = _PyWeakref_GET_REF((PyObject*)self); if (obj == NULL) { PyErr_SetString(PyExc_TypeError, "weak object has gone away"); return -1; } - self->hash = PyObject_Hash(obj); + hash = PyObject_Hash(obj); Py_DECREF(obj); - return self->hash; -} - -static Py_hash_t -weakref_hash(PyObject *op) -{ - PyWeakReference *self = _PyWeakref_CAST(op); - Py_hash_t hash; - Py_BEGIN_CRITICAL_SECTION(self); - hash = weakref_hash_lock_held(self); - Py_END_CRITICAL_SECTION(); + _Py_atomic_store_ssize_relaxed(&self->hash, hash); return hash; } @@ -353,7 +379,13 @@ try_reuse_basic_ref(PyWeakReference *list, PyTypeObject *type, cand = proxy; } - if (cand != NULL && _Py_TryIncref((PyObject *) cand)) { + if (cand == NULL) { + return NULL; + } + PyObject* candobj = _PyObject_CAST(cand); + int incref_res = _Py_IsImmutable(candobj) ? + _Py_TryIncref_Immutable(candobj) : _Py_TryIncref(candobj); + if (incref_res) { return cand; } return NULL; diff --git a/Python/immutability.c b/Python/immutability.c index 2c99bc0bf295a1..06cc07e3e1b5e6 100644 --- a/Python/immutability.c +++ b/Python/immutability.c @@ -1791,6 +1791,48 @@ void _Py_RefcntAdd_Immutable(PyObject *op, Py_ssize_t increment) #endif } +/* Tries to incref op and returns 1 if successful or 0 otherwise. + * Used when creating a strong reference from a weak reference. + * Needs to hold the weakref list lock (LOCK_WEAKREFS). + */ +int _Py_TryIncref_Immutable(PyObject *op) +{ + assert(_Py_IsImmutable(op)); + op = scc_root(op); + assert(_Py_IsImmutable(op)); + +#if SIZEOF_VOID_P > 4 + uint32_t old = _Py_atomic_load_uint32_relaxed(&op->ob_refcnt); + while (old > 0) { + if (_Py_atomic_compare_exchange_uint32(&op->ob_refcnt, &old, old + 1)) { + return 1; + } + } +#else + Py_ssize_t old = _Py_atomic_load_ssize_relaxed(&op->ob_refcnt); + while (_Py_IMMUTABLE_FLAG_CLEAR(old) != 0) { + if (_Py_atomic_compare_exchange_ssize(&op->ob_refcnt, &old, old + 1)) { + return 1; + } + } +#endif + return 0; +} + +/* Returns 1 if there are no references to the object's SCC. */ +int _Py_IsDead_Immutable(PyObject *op) +{ + assert(_Py_IsImmutable(op)); + op = scc_root(op); + assert(_Py_IsImmutable(op)); + +#if SIZEOF_VOID_P > 4 + return _Py_atomic_load_uint32_relaxed(&op->ob_refcnt) == 0; +#else + return _Py_IMMUTABLE_FLAG_CLEAR(_Py_atomic_load_ssize_relaxed(&op->ob_refcnt)) == 0; +#endif +} + // Macro that jumps to error, if the expression `x` does not succeed. #define SUCCEEDS(x) { do { int r = (x); if (r != 0) goto error; } while (0); } From d647ae7ee6906f835dd685180d58da396d97129f Mon Sep 17 00:00:00 2001 From: David Klement Date: Wed, 11 Mar 2026 14:53:23 +0100 Subject: [PATCH 3/5] Immutability: Reference counting for weakrefs --- Include/internal/pycore_weakref.h | 1 + Objects/weakrefobject.c | 54 ++++++++++++++++++++++++++++++- Python/immutability.c | 36 +++++++++++++++++++++ 3 files changed, 90 insertions(+), 1 deletion(-) diff --git a/Include/internal/pycore_weakref.h b/Include/internal/pycore_weakref.h index 64854090474781..cde4ea27884603 100644 --- a/Include/internal/pycore_weakref.h +++ b/Include/internal/pycore_weakref.h @@ -149,6 +149,7 @@ extern Py_ssize_t _PyWeakref_GetWeakrefCount(PyObject *obj); // intact. extern void _PyWeakref_ClearWeakRefsNoCallbacks(PyObject *obj); +PyAPI_FUNC(void) _PyWeakref_OnObjectFreeze(PyObject *object); PyAPI_FUNC(int) _PyWeakref_IsDead(PyObject *weakref); #ifdef __cplusplus diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index 9db7c7eab1f1a0..e2b5b0ec1a5284 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -55,10 +55,21 @@ * lock is held. * * We also need to handle refcounts for the weakref object and the callback. - * TODO(Immutable-weakrefs): Implement this; handle weakrefs with callbacks. * * - Basic weakrefs pointing to immutable objects are marked as immutable, * which turns on atomic reference counting. + * - Weakrefs with callbacks and pointing to immutable objects + * have their refcount pre-emptively incremented upon creation. + * That accounts for the TryIncref that would be called when clearing + * weakrefs, which would require atomic reference counting. + * However, we cannot easily achieve atomic reference counting for weakrefs + * with callbacks: we cannot make them immutable, and adding another branch + * to PY_INCREF and PY_DECREF would have a significant performance impact. + * The downside of our approach is that the weakref objects are kept alive + * until the immutable object dies. + * FIXME(Immutable): If the weakref is a part of an SCC, it never dies. + * - We keep the callback in the weakref object until it is about to be called. + * That keeps it alive, so we don't need to increment its refcount. * * Immutable objects are never GC-collected. */ @@ -435,6 +446,44 @@ insert_weakref(PyWeakReference *newref, PyWeakReference **list) } } +static void +immutable_make_weakref_safe(PyWeakReference *self) +{ + if (self->wr_callback == NULL) { + // Turn on atomic reference counting for the weakref. + // FIXME(Immutable): freezing a weakref makes it strong + // _PyImmutability_Freeze(_PyObject_CAST(newref)); + } + else { + // Pre-emptively increment the weakref's refcount. + // See the comment at the start of this file for details. + Py_INCREF(self); + } + +} + +/* Make weakrefs to the newly frozen object thread-safe. */ +void +_PyWeakref_OnObjectFreeze(PyObject *object) +{ + assert(_Py_IsImmutable(object)); + if (!_PyType_SUPPORTS_WEAKREFS(Py_TYPE(object))) { + return; + } + PyWeakReference **list = GET_WEAKREFS_LISTPTR(object); + if (_Py_atomic_load_ptr(list) == NULL) { + // Fast path for the common case + return; + } + LOCK_WEAKREFS(object); + PyWeakReference *current = *list; + while (current != NULL) { + immutable_make_weakref_safe(current); + current = current->wr_next; + } + UNLOCK_WEAKREFS(object); +} + static PyWeakReference * allocate_weakref(PyTypeObject *type, PyObject *obj, PyObject *callback) { @@ -443,6 +492,9 @@ allocate_weakref(PyTypeObject *type, PyObject *obj, PyObject *callback) return NULL; } init_weakref(newref, obj, callback); + if (_Py_IsImmutable(obj)) { + immutable_make_weakref_safe(newref); + } return newref; } diff --git a/Python/immutability.c b/Python/immutability.c index 06cc07e3e1b5e6..bacd277aca4bdc 100644 --- a/Python/immutability.c +++ b/Python/immutability.c @@ -8,6 +8,7 @@ #include "pycore_object.h" #include "pycore_immutability.h" #include "pycore_list.h" +#include "pycore_weakref.h" // This file has many in progress aspects @@ -1833,6 +1834,40 @@ int _Py_IsDead_Immutable(PyObject *op) #endif } +static void make_weakrefs_safe_scc(PyObject* scc) +{ + PyObject* current = scc; + do { + _PyWeakref_OnObjectFreeze(current); + current = scc_next(current); + } while (current != NULL && current != scc); +} + +static int make_weakrefs_safe_visited( + _Py_hashtable_t* tbl, const void* key, const void* value, void* unused) +{ + (void)tbl; + (void)value; + (void)unused; + _PyWeakref_OnObjectFreeze((PyObject*)key); + return 0; +} + +/* Make weakrefs to newly frozen objects thread-safe. */ +static void make_weakrefs_safe(struct FreezeState* freeze_state) +{ + // Handle weakrefs to completed SCCs. + PyObject *scc = freeze_state->completed_sccs; + while (scc != NULL) { + PyObject *next = scc_parent(scc); + make_weakrefs_safe_scc(scc); + scc = next; + } + // Handle weakrefs to non-GC visited objects. + _Py_hashtable_foreach(freeze_state->visited, + make_weakrefs_safe_visited, NULL); + +} // Macro that jumps to error, if the expression `x` does not succeed. #define SUCCEEDS(x) { do { int r = (x); if (r != 0) goto error; } while (0); } @@ -2061,6 +2096,7 @@ freeze_impl(PyObject *const *objs, Py_ssize_t nobjs) SUCCEEDS(traverse_freeze(item, &freeze_state)); } + make_weakrefs_safe(&freeze_state); mark_all_frozen(&freeze_state); goto finally; From 0af935653ca5d549f14083e07be07c4ff19efdb2 Mon Sep 17 00:00:00 2001 From: David Klement Date: Fri, 6 Mar 2026 11:50:41 +0100 Subject: [PATCH 4/5] Immutability: A few more weakref tests --- Lib/test/test_freeze/test_core.py | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_freeze/test_core.py b/Lib/test/test_freeze/test_core.py index 57b6feb99bfa60..decec086985d32 100644 --- a/Lib/test/test_freeze/test_core.py +++ b/Lib/test/test_freeze/test_core.py @@ -1,5 +1,6 @@ import unittest from immutable import freeze, NotFreezable, isfrozen +import weakref from .test_common import BaseNotFreezableTest, BaseObjectTest @@ -445,12 +446,11 @@ class B: class C: # Function that takes a object, and stores it in a weakref field. def __init__(self, obj): - import weakref self.obj = weakref.ref(obj) def val(self): return self.obj() - def test_weakref(self): + def test_freeze_object_with_weakref(self): obj = TestWeakRef.B() c = TestWeakRef.C(obj) freeze(c) @@ -461,6 +461,30 @@ def test_weakref(self): # The reference should remain as it was reachable through a frozen weakref. self.assertTrue(c.val() is not None) + def test_weakref_to_frozen_object(self): + obj = TestWeakRef.B() + freeze(obj) + c = TestWeakRef.C(obj) + self.assertTrue(isfrozen(c.val())) + # TODO(Immutable): Do we want to freeze the weakref? + # self.assertTrue(isfrozen(c.obj)) + + def test_remove_weakref(self): + obj = TestWeakRef.B() + c = TestWeakRef.C(obj) + freeze(c) + obj = None + # The reference should be removed + self.assertTrue(weakref.getweakrefcount(obj) == 0) + + def test_reuse_weakref(self): + obj = TestWeakRef.B() + freeze(obj) + c1 = TestWeakRef.C(obj) + c2 = TestWeakRef.C(obj) + # The weakrefs should be the same, as they refer to the same object + self.assertTrue(c1.obj is c2.obj) + class TestStackCapture(unittest.TestCase): def test_stack_capture(self): import sys From ec30f379104d8490327cb1148ff66ca7cf8c6672 Mon Sep 17 00:00:00 2001 From: David Klement Date: Wed, 11 Mar 2026 17:42:13 +0100 Subject: [PATCH 5/5] Immutability: Call weakref callbacks --- Include/cpython/weakrefobject.h | 5 + Include/internal/pycore_weakref.h | 1 + Objects/weakrefobject.c | 53 ++++++ Python/immutability.c | 260 +++++++++++++++++++++++++++++- 4 files changed, 313 insertions(+), 6 deletions(-) diff --git a/Include/cpython/weakrefobject.h b/Include/cpython/weakrefobject.h index e0711407cee470..55f8e046e35e14 100644 --- a/Include/cpython/weakrefobject.h +++ b/Include/cpython/weakrefobject.h @@ -16,6 +16,11 @@ struct _PyWeakReference { /* A callable to invoke when wr_object dies, or NULL if none. */ PyObject *wr_callback; + /* ID of the interpreter where the callback resides. + * Used for immutable objects to know which interpreter to call back into. + * This is -1 if the callback is NULL. + */ + int64_t callback_ipid; /* A cache for wr_object's hash code. As usual for hashes, this is -1 * if the hash code isn't known yet. diff --git a/Include/internal/pycore_weakref.h b/Include/internal/pycore_weakref.h index cde4ea27884603..e46829c4d39b32 100644 --- a/Include/internal/pycore_weakref.h +++ b/Include/internal/pycore_weakref.h @@ -150,6 +150,7 @@ extern Py_ssize_t _PyWeakref_GetWeakrefCount(PyObject *obj); extern void _PyWeakref_ClearWeakRefsNoCallbacks(PyObject *obj); PyAPI_FUNC(void) _PyWeakref_OnObjectFreeze(PyObject *object); +PyAPI_FUNC(void) _PyImmutability_ClearWeakRefs(PyObject *object, PyWeakReference **callbacks); PyAPI_FUNC(int) _PyWeakref_IsDead(PyObject *weakref); #ifdef __cplusplus diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index e2b5b0ec1a5284..bb0ffb1dd5eee3 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -71,6 +71,13 @@ * - We keep the callback in the weakref object until it is about to be called. * That keeps it alive, so we don't need to increment its refcount. * + * Calling the callback is tricky because it can reside on a different + * interpreter than the interpreter that triggered deallocation. + * Therefore, we keep track of the original interpreter of the callback. + * When the immutable object is being deallocated, we schedule the callbacks + * to be called on their original interpreters using an asynchronous call. + * Once all callbacks have been called, we continue deallocating the object. + * * Immutable objects are never GC-collected. */ #endif @@ -109,6 +116,12 @@ init_weakref(PyWeakReference *self, PyObject *ob, PyObject *callback) self->wr_prev = NULL; self->wr_next = NULL; self->wr_callback = Py_XNewRef(callback); + if (callback == NULL) { + self->callback_ipid = -1; + } + else { + self->callback_ipid = PyInterpreterState_GetID(PyInterpreterState_Get()); + } self->vectorcall = weakref_vectorcall; #ifdef Py_GIL_DISABLED self->weakrefs_lock = &WEAKREF_LIST_LOCK(ob); @@ -1076,6 +1089,7 @@ PyWeakref_GetObject(PyObject *ref) /* Note that there's an inlined copy-paste of handle_callback() in gcmodule.c's * handle_weakrefs(). + * There is also a copy-paste in immutability.c. */ static void handle_callback(PyWeakReference *ref, PyObject *callback) @@ -1104,6 +1118,7 @@ PyObject_ClearWeakRefs(PyObject *object) if (object == NULL || !_PyType_SUPPORTS_WEAKREFS(Py_TYPE(object)) + || _Py_IsImmutable(object) || Py_REFCNT(object) != 0) { PyErr_BadInternalCall(); @@ -1181,6 +1196,44 @@ PyObject_ClearWeakRefs(PyObject *object) PyErr_SetRaisedException(exc); } +/* Clear weak references of an immutable object. + * For weakrefs with callbacks, add them to the callbacks list + * to be handled later. + */ +void +_PyImmutability_ClearWeakRefs(PyObject *object, PyWeakReference **callbacks) +{ + if (object == NULL + || !_PyType_SUPPORTS_WEAKREFS(Py_TYPE(object)) + || !_Py_IsImmutable(object)) + { + PyErr_BadInternalCall(); + return; + } + + PyWeakReference **list = GET_WEAKREFS_LISTPTR(object); + if (_Py_atomic_load_ptr(list) == NULL) { + // Fast path for the common case + return; + } + + // Weakrefs without callbacks are just cleared. + // Weakrefs with callbacks are cleared and added to the list. + for (int done = 0; !done;) { + LOCK_WEAKREFS(object); + PyWeakReference *cur = *list; + if (cur != NULL) { + clear_weakref_lock_held(cur, NULL); // keeps wr_callback + if (cur->wr_callback != NULL) { + // We need to store the callback to be able to call it later. + insert_head(cur, callbacks); + } + } + done = (*list == NULL); + UNLOCK_WEAKREFS(object); + } +} + void PyUnstable_Object_ClearWeakRefsNoCallbacks(PyObject *obj) { diff --git a/Python/immutability.c b/Python/immutability.c index bacd277aca4bdc..9cf8fef9f9e808 100644 --- a/Python/immutability.c +++ b/Python/immutability.c @@ -7,6 +7,7 @@ #include "pycore_gc.h" #include "pycore_object.h" #include "pycore_immutability.h" +#include "pycore_interp.h" #include "pycore_list.h" #include "pycore_weakref.h" @@ -800,6 +801,242 @@ static void unfreeze(PyObject* obj) scc_return_to_gc(obj, true); } +// Copy-pasted from weakrefobject.c +static void weakref_handle_callback(PyWeakReference* ref, PyObject* callback) +{ + PyObject* cbresult = PyObject_CallOneArg(callback, (PyObject*)ref); + + if (cbresult == NULL) { + PyErr_FormatUnraisable("Exception ignored while " + "calling weakref callback %R", callback); + } + else { + Py_DECREF(cbresult); + } +} + +// Copy-pasted from weakrefobject.c +static void weakref_insert_head(PyWeakReference* newref, PyWeakReference** list) +{ + PyWeakReference* next = *list; + + newref->wr_prev = NULL; + newref->wr_next = next; + if (next != NULL) + next->wr_prev = newref; + *list = newref; +} + +static void weakref_remove(PyWeakReference* self, PyWeakReference** list) +{ + if (*list == self) { + *list = self->wr_next; + } + if (self->wr_prev != NULL) { + self->wr_prev->wr_next = self->wr_next; + } + if (self->wr_next != NULL) { + self->wr_next->wr_prev = self->wr_prev; + } + self->wr_prev = NULL; + self->wr_next = NULL; +} + +static void weakref_decref_weakrefs(PyWeakReference* head) +{ + while (head != NULL) { + PyWeakReference* weakref = head; + head = weakref->wr_next; + weakref->wr_next = NULL; + weakref->wr_prev = NULL; + Py_DECREF(weakref); + } +} + +typedef struct { + int32_t interpreters_remaining; + PyObject* to_dealloc; +} callback_progress; + +typedef struct { + PyWeakReference* head; + callback_progress* progress; +} pending_callbacks; + +/* Signal that the current interpreter handled the callbacks. + * If all interpreters have handled the callbacks, deallocate the object. + */ +static void weakref_signal_handled(callback_progress* progress) +{ + int32_t old = _Py_atomic_add_int32( + &progress->interpreters_remaining, -1); + if (old == 1) { + // All callbacks handled, trigger deallocation again. + Py_INCREF(progress->to_dealloc); + Py_DECREF(progress->to_dealloc); + PyMem_Free(progress); + } +} + +/* Call the pending callbacks. + * This function can be executed asynchronously using Py_AddPendingCall. + */ +static int weakref_call_callbacks(void* arg) +{ + pending_callbacks* pending = (pending_callbacks*)arg; + PyWeakReference* head = pending->head; + debug("Interpreter %zd handling callbacks for dying object %p\n", + PyInterpreterState_GetID(PyInterpreterState_Get()), + pending->progress->to_dealloc); + + while (head != NULL) { + PyWeakReference* weakref = head; + PyObject* callback = weakref->wr_callback; + assert(callback != NULL); + weakref->wr_callback = NULL; + weakref_handle_callback(weakref, callback); + Py_DECREF(callback); + head = weakref->wr_next; + weakref->wr_next = NULL; + weakref->wr_prev = NULL; + Py_DECREF(weakref); + } + + weakref_signal_handled(pending->progress); + PyMem_Free(pending); + // Report success as per Py_AddPendingCall contract + return 0; +} + +/* Schedule the callbacks on the given interpreter. */ +static void weakref_schedule_callbacks(int64_t ipid, pending_callbacks* pending) +{ + // FIXME(Immutable-weakrefs): Can the interpreter go away in the middle of scheduling? + PyInterpreterState* target_is = _PyInterpreterState_LookUpID(ipid); + if (target_is == NULL) { + // Interpreter is already gone. + goto abort; + } + // We just need to get any thread state to schedule the call. + PyThreadState* tstate_target = PyInterpreterState_ThreadHead(target_is); + if (tstate_target == NULL) { + goto abort; + } + PyThreadState* tstate_old = PyThreadState_Swap(tstate_target); + int schedule_res = Py_AddPendingCall(weakref_call_callbacks, (void*)pending); + PyThreadState_Swap(tstate_old); + if (schedule_res != 0) { + goto abort; + } + return; + +abort: + PyMem_Free(pending); + weakref_decref_weakrefs(pending->head); + return; +} + +/* Remove callbacks with the given ipid from the list. + * Return them as a new list. + */ +static PyWeakReference* weakref_separate_ipid(PyWeakReference** list, int64_t ipid) +{ + PyWeakReference* result = NULL; + PyWeakReference* next = *list; + while (next != NULL) { + PyWeakReference* current = next; + next = next->wr_next; + if (current->callback_ipid == ipid) { + weakref_remove(current, list); + weakref_insert_head(current, &result); + } + } + return result; +} + +/* Clear the weakrefs in the list. + * Returns: + * (true) The caller can proceed with deallocating 'to_dealloc'. + * (false) Callbacks were scheduled, deallocation will be triggered again. + */ +static int clear_weakrefs(PyWeakReference* head, PyObject* to_dealloc) +{ + if (head == NULL) { + return true; + } + + debug("Clearing weakrefs of %p.\n", to_dealloc); + // We want to continue with deallocation after calling all the callbacks. + callback_progress* progress = PyMem_Malloc(sizeof(callback_progress)); + if (progress == NULL) { + // Give up calling callbacks. + weakref_decref_weakrefs(head); + return true; + } + // Start with 1, decremented at the end of this function. + // This way, we prevent hitting zero before all callbacks are scheduled. + progress->interpreters_remaining = 1; + progress->to_dealloc = to_dealloc; + + // Schedule the callbacks on their original interpreters. + while (head != NULL) { + int64_t ipid = head->callback_ipid; + PyWeakReference* ip_callbacks = weakref_separate_ipid(&head, ipid); + // Create a data structure to hold arguments for the async call. + pending_callbacks* pending = PyMem_Malloc(sizeof(pending_callbacks)); + if (pending == NULL) { + // Give up calling callbacks. + weakref_decref_weakrefs(ip_callbacks); + } + else { + _Py_atomic_add_int32(&progress->interpreters_remaining, 1); + pending->head = ip_callbacks; + pending->progress = progress; + // TODO(Immutable-weakrefs): run immediately if ipid is the current interpreter + weakref_schedule_callbacks(ipid, pending); + } + } + + weakref_signal_handled(progress); + return false; +} + +/* Clear the weakrefs for an SCC. + * Returns: + * (true) Deallocation can continue. + * (false) Callbacks were scheduled, deallocation will be triggered again. + */ +static int clear_weakrefs_scc(PyObject* obj) +{ + // Collect weakrefs with callbacks into a list. + PyWeakReference* head = NULL; + PyObject* n = obj; + do { + PyObject* c = n; + n = scc_next(c); + if (_PyType_SUPPORTS_WEAKREFS(Py_TYPE(c))) { + _PyImmutability_ClearWeakRefs(c, &head); + } + } while (n != obj); + + return clear_weakrefs(head, obj); +} + +/* Clear the weakrefs for a single object. + * Returns: + * (true) Deallocation can continue. + * (false) Callbacks were scheduled, deallocation will be triggered again. + */ +static int clear_weakrefs_single(PyObject* obj) +{ + if (!_PyType_SUPPORTS_WEAKREFS(Py_TYPE(obj))) { + return true; + } + // Collect weakrefs with callbacks into a list. + PyWeakReference* head = NULL; + _PyImmutability_ClearWeakRefs(obj, &head); + return clear_weakrefs(head, obj); +} static void unfreeze_and_finalize_scc(PyObject* obj) { @@ -809,12 +1046,15 @@ static void unfreeze_and_finalize_scc(PyObject* obj) scc_set_refcounts_to_one(obj); scc_add_internal_refcounts(obj, &scc_details); - // These are cases that we don't handle. Return the state as mutable to the - // cycle detector to handle. - // TODO(Immutable): Lift the weak references to be handled here. - if (scc_details.has_weakreferences > 0 || scc_details.has_legacy_finalizers > 0) { - debug("There are weak references or legacy finalizers in the SCC. Let cycle detector handle this case.\n"); - debug("Weak references: %d, Legacy finalizers: %d\n", scc_details.has_weakreferences, scc_details.has_legacy_finalizers); + if (scc_details.has_weakreferences > 0) { + // TODO(Immutable-weakrefs): We don't need this anymore? + } + + // We don't handle legacy finalizers. + // Return the state as mutable to the cycle detector to handle. + if (scc_details.has_legacy_finalizers > 0) { + debug("There are legacy finalizers in the SCC. Let cycle detector handle this case.\n"); + debug("Legacy finalizers: %d\n", scc_details.has_legacy_finalizers); scc_make_mutable(obj); scc_return_to_gc(obj, true); return; @@ -1749,6 +1989,10 @@ int _Py_DecRef_Immutable(PyObject *op) if (scc_next(op) != NULL) { // This is part of an SCC, so we need to turn it back into mutable state, // and correctly re-establish RCs. + if (!clear_weakrefs_scc(op)) { + // Callbacks were scheduled, deallocation will be triggered again. + return false; + } unfreeze_and_finalize_scc(op); return false; } @@ -1757,6 +2001,10 @@ int _Py_DecRef_Immutable(PyObject *op) return_to_gc(op); } + if (!clear_weakrefs_single(op)) { + // Callbacks were scheduled, deallocation will be triggered again. + return false; + } _Py_CLEAR_IMMUTABLE(op); if (PyWeakref_Check(op)) {