diff --git a/Include/internal/pycore_cown.h b/Include/internal/pycore_cown.h new file mode 100644 index 000000000000000..0345690cc015ab6 --- /dev/null +++ b/Include/internal/pycore_cown.h @@ -0,0 +1,29 @@ +#ifndef Py_INTERNAL_COWN_H +#define Py_INTERNAL_COWN_H +#ifdef __cplusplus +extern "C" { +#endif + +#ifndef Py_BUILD_CORE +# error "Py_BUILD_CORE must be defined to include this header" +#endif + +#include "object.h" +#include "exports.h" + +typedef struct _PyCownObject _PyCownObject; +#define _PyCownObject_CAST(op) _Py_CAST(_PyCownObject*, op) + +PyAPI_DATA(PyTypeObject) _PyCown_Type; + +typedef uint64_t _PyCown_ipid_t; +typedef uint64_t _PyCown_thread_id_t; + +PyAPI_FUNC(_PyCown_ipid_t) _PyCown_ThisInterpreterId(void); +PyAPI_FUNC(_PyCown_thread_id_t) _PyCown_ThisThreadId(void); + + +#ifdef __cplusplus +} +#endif +#endif /* !Py_INTERNAL_COWN_H */ \ No newline at end of file diff --git a/Include/internal/pycore_gc.h b/Include/internal/pycore_gc.h index 2dfce32237a83c3..6a1f91d2bad7cde 100644 --- a/Include/internal/pycore_gc.h +++ b/Include/internal/pycore_gc.h @@ -352,6 +352,25 @@ extern PyObject *_PyGC_GetObjects(PyInterpreterState *interp, int generation); extern PyObject *_PyGC_GetReferrers(PyInterpreterState *interp, PyObject *objs); // Functions to clear types free lists +/* Disposal of a list of objects that are known to be unreachable. Used by the + * collector itself and by anything else that owns a set of objects it has + * established to be garbage, such as a closed tracing region. + * + * `_PyGC_FinalizeGarbage()` runs the finalizer of every object in `collectable`, + * before anything is cleared, so that a `__del__` still sees its object intact. + * + * `_PyGC_DeleteGarbage()` then breaks the references between them, deallocating + * every object whose reference count reaches zero. Objects that a finalizer kept + * alive are moved to `old` instead. + * + * Neither may be called with an exception set. Only available in the default + * build; the free-threaded collector has its own implementation. + */ +#ifndef Py_GIL_DISABLED +extern void _PyGC_FinalizeGarbage(PyGC_Head *collectable); +extern void _PyGC_DeleteGarbage(PyGC_Head *collectable, PyGC_Head *old); +#endif + extern void _PyGC_ClearAllFreeLists(PyInterpreterState *interp); extern void _Py_ScheduleGC(PyThreadState *tstate); extern void _Py_RunGC(PyThreadState *tstate); diff --git a/Include/internal/pycore_immutability.h b/Include/internal/pycore_immutability.h index 8e4d32b78527a63..32b49580c56d775 100644 --- a/Include/internal/pycore_immutability.h +++ b/Include/internal/pycore_immutability.h @@ -8,6 +8,10 @@ extern "C" { # error "Py_BUILD_CORE must be defined to include this header" #endif +PyAPI_DATA(PyTypeObject) _PyTracingRegion_Type; +PyAPI_FUNC(int) _PyTracingRegion_Close(PyObject* region); +PyAPI_FUNC(int) _PyTracingRegion_IsClosed(PyObject* region); + struct _Py_immutability_state { int late_init_done; struct _Py_hashtable_t *shallow_immutable_types; diff --git a/Lib/immutable.py b/Lib/immutable.py index e1c00152f94bbd8..167273bc31cdbd6 100644 --- a/Lib/immutable.py +++ b/Lib/immutable.py @@ -21,6 +21,8 @@ FREEZABLE_PROXY = _c.FREEZABLE_PROXY InterpreterLocal = _c.InterpreterLocal SharedField = _c.SharedField +TracingRegion = _c.TracingRegion +Cown = _c.Cown # FIXME(immutable): For the longest time we used the name `isfrozen` # without the underscore. This keeps the function name for now, but diff --git a/Lib/test/test_freeze/test_implicit.py b/Lib/test/test_freeze/test_implicit.py index b710b787fe5bdaf..35e46036e66d75f 100644 --- a/Lib/test/test_freeze/test_implicit.py +++ b/Lib/test/test_freeze/test_implicit.py @@ -1,3 +1,4 @@ +import sys import unittest from immutable import freeze, is_frozen @@ -139,6 +140,26 @@ def test_deeply_nested_no_stack_overflow(self): obj = (obj,) self.assertTrue(is_frozen(obj)) + def test_abandoned_walk_keeps_references(self): + """An aborted walk must not drop references it never took. + + The walk pushes objects onto a worklist without increfing them, so + anything still on the worklist when a mutable object aborts the walk + used to be decrefed when the worklist was released. That freed the + object while its real owners were still pointing at it, which showed + up much later as a negative refcount. + """ + # Built at runtime so it is neither interned nor immortal, which makes + # its reference count fully accounted for by this test. + item = "".join(["abandoned", "-", "worklist", "-", "entry"]) + # Tuples are traversed back to front, so `item` reaches the worklist + # before the dict aborts the walk. + obj = ({"mutable": 1}, item) + + before = sys.getrefcount(item) + self.assertFalse(is_frozen(obj)) + self.assertEqual(sys.getrefcount(item), before) + if __name__ == '__main__': unittest.main() diff --git a/Lib/test/test_freeze/test_tracing_region.py b/Lib/test/test_freeze/test_tracing_region.py new file mode 100644 index 000000000000000..f661bf0fa8b83cd --- /dev/null +++ b/Lib/test/test_freeze/test_tracing_region.py @@ -0,0 +1,253 @@ +import re +import sys +import unittest +from immutable import freeze, is_frozen, freezable +from immutable import TracingRegion as Region +from immutable import Cown + +def sort_region_error(msg): + """Normalize a 'region could not be closed' message by masking the object + addresses and sorting its per-object lines. Useful for deterministic test + assertions, since the addresses differ per run and the object order comes + from hashtable iteration and isn't stable.""" + header, *lines = re.sub(r"0x[0-9a-fA-F]+", "0x...", msg).splitlines() + return [header, *sorted(lines)] + +class TestTraceRefs(unittest.TestCase): + def test_release_error(self): + x = [1] + y = [2] + + c = Cown(Region()) + c.value.x = x + c.value.y = y + + with self.assertRaises(RuntimeError) as cm: + c.release() + + self.assertEqual( + sort_region_error(str(cm.exception)), + [ + "The region could not be closed due to:", + "- 1 incoming reference to list '[1]'", + "- 1 incoming reference to list '[2]'" + ]) + + def test_release_error_capped_output(self): + # The object order in the error message is based on the address + # and therefore fairly random. All elements look the same of + # make testing stable. + l = [[1], [1], [1], [1], [1], [1], [1], [1]] + + c = Cown(Region()) + c.value.x = [] + + for i in range(len(l)): + c.value.x.append(l[i]) + + with self.assertRaises(RuntimeError) as cm: + c.release() + + self.assertEqual( + sort_region_error(str(cm.exception)), + [ + "The region could not be closed due to:", + "- 1 incoming reference to list '[1]'", + "- 1 incoming reference to list '[1]'", + "- 1 incoming reference to list '[1]'", + "- 1 incoming reference to list '[1]'", + "- 1 incoming reference to list '[1]'", + "- 3 references to other objects", + ]) + + # The cown should now be released + l = None + c.release() + + def test_release_error_in_subregion(self): + x = [1] + + c = Cown(Region()) + child = Region() + child.x = x + c.value.child = child + + with self.assertRaises(RuntimeError) as cm: + c.release() + + self.assertEqual( + sort_region_error(str(cm.exception)), + [ + "The region could not be closed due to:", + "- 1 incoming reference to list '[1]'", + ]) + + +class TestRegionOpening(unittest.TestCase): + def test_open_after_acquire(self): + c = Cown(Region()) + c.value.x = [] + self.assertFalse(c._is_closed()) + + c.release() + c.acquire() + + self.assertTrue(c._is_closed()) + c.value.x = None + self.assertFalse(c._is_closed()) + + def test_release_closed_region(self): + c = Cown(Region()) + c.value.x = [] + self.assertFalse(c._is_closed()) + + c.release() + c.acquire() + + self.assertTrue(c._is_closed()) + + c.release() + + def test_bridge_refs_keep_region_closed(self): + c = Cown(Region()) + c.release() + c.acquire() + self.assertTrue(c._is_closed()) + + # Adding new references to the bridge object should keep it closed. + # only attribute accesses should open it. + r1 = c.value + r2 = c.value + self.assertTrue(c._is_closed()) + + # However, these references should prevent the cown from being released + with self.assertRaises(RuntimeError) as cm: + c.release() + + self.assertEqual( + str(cm.exception), + "the cown couldn't be released, due to the bridge having incoming references") + + # The release should succeed once all refs have been killed + del r1 + del r2 + c.release() + + def test_sub_region_closing(self): + @freezable + class A: + pass + c = Cown(Region()) + c.value.a = A() + c.value.a.child = Region() + c.value.a.child.b = A() + + c.release() + c.acquire() + + r2 = c.value.a.child + c2 = Cown(r2) + + self.assertTrue(c2._is_closed()) + + def test_sub_region_multiple_refs(self): + @freezable + class A: + pass + c = Cown(Region()) + c.value.a = A() + sub = Region() + c.value.a.child_a = sub + c.value.a.child_b = sub + # A reference to the bridge of a sub-region counts as an incoming + # reference into the parent region, see + # test_ref_to_sub_region_bridge_keeps_parent_open. + del sub + + c.release() + c.acquire() + + r2 = c.value.a.child_a + c2 = Cown(r2) + + self.assertTrue(c2._is_closed()) + + def test_ref_to_sub_region_bridge_keeps_parent_open(self): + c1 = Cown(Region()) + c2 = Cown(Region()) + c1.value.child = c2.value + + self.assertFalse(c2._is_closed()) + + with self.assertRaises(RuntimeError) as cm: + c1.release() + + # Attempting to close the region c1 should have closed c2 and then + # failed due to the incoming reference to the bridge stored in c2 + self.assertTrue(c2._is_closed()) + + + self.assertEqual( + sort_region_error(str(cm.exception)), + [ + "The region could not be closed due to:", + "- 1 incoming reference to TracingRegion ''", + ]) + + + +class TestImplicitFreeze(unittest.TestCase): + def test_implicit_freeze_func(self): + @freezable + def some_func(): + pass + c = Cown(Region()) + + c.value.obj = some_func + self.assertFalse(is_frozen(c.value.obj)) + c.release() + c.acquire() + self.assertTrue(is_frozen(c.value.obj)) + + def test_implicit_freeze_type(self): + @freezable + class A: + pass + c = Cown(Region()) + + c.value.obj = A + self.assertFalse(is_frozen(c.value.obj)) + c.release() + c.acquire() + self.assertTrue(is_frozen(c.value.obj)) + + def test_implicit_freeze_module(self): + import random; + c = Cown(Region()) + + c.value.obj = random + self.assertFalse(is_frozen(c.value.obj)) + c.release() + c.acquire() + self.assertTrue(is_frozen(c.value.obj)) + + # Unimport module + sys.modules.pop("random", None) + sys.mut_modules.pop("random", None) + + def test_implicit_freeze_str(self): + c = Cown(Region()) + + c.value.obj = "Ducks are cool" + c.release() + c.acquire() + self.assertTrue(is_frozen(c.value.obj)) + + def test_implicit_freeze_int(self): + c = Cown(Region()) + + c.value.obj = 17 + c.release() + c.acquire() + self.assertTrue(is_frozen(c.value.obj)) + diff --git a/Makefile.pre.in b/Makefile.pre.in index 572a784546b60fb..d8ae75ed97237e0 100644 --- a/Makefile.pre.in +++ b/Makefile.pre.in @@ -528,6 +528,7 @@ OBJECT_OBJS= \ Objects/classobject.o \ Objects/codeobject.o \ Objects/complexobject.o \ + Objects/cownobject.o \ Objects/descrobject.o \ Objects/enumobject.o \ Objects/exceptions.o \ @@ -555,6 +556,7 @@ OBJECT_OBJS= \ Objects/sliceobject.o \ Objects/structseq.o \ Objects/templateobject.o \ + Objects/tracingregionobject.o \ Objects/tupleobject.o \ Objects/typeobject.o \ Objects/typevarobject.o \ diff --git a/Modules/_immutablemodule.c b/Modules/_immutablemodule.c index 94ea460c340526e..2c5a5ac665a5108 100644 --- a/Modules/_immutablemodule.c +++ b/Modules/_immutablemodule.c @@ -8,6 +8,7 @@ #include "Python.h" #include +#include "pycore_cown.h" #include "pycore_object.h" #include "pycore_immutability.h" #include "pycore_critical_section.h" @@ -650,6 +651,21 @@ immutable_exec(PyObject *module) { return -1; } + if (PyModule_AddType(module, &_PyTracingRegion_Type) != 0) { + return -1; + } + if (_PyImmutability_SetFreezable( + (PyObject*)&_PyTracingRegion_Type, _Py_FREEZABLE_YES) < 0) { + return -1; + } + + if (PyModule_AddType(module, &_PyCown_Type) != 0) { + return -1; + } + if (_PyImmutability_SetFreezable((PyObject*)&_PyCown_Type, _Py_FREEZABLE_YES) < 0) { + return -1; + } + if (PyModule_AddIntConstant(module, "FREEZABLE_YES", _Py_FREEZABLE_YES) != 0) { return -1; diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c index c73e79eec243fd6..81f93c8332485bb 100644 --- a/Modules/_sqlite/connection.c +++ b/Modules/_sqlite/connection.c @@ -155,6 +155,11 @@ static PyObject * new_statement_cache(pysqlite_Connection *self, pysqlite_state *state, int maxsize) { + // FIXME(regions): statement cache disabled for testing. Return the connection + // itself (its tp_call creates a fresh statement) so callers of + // statement_cache(sql) bypass the functools.lru_cache wrapper. + return Py_NewRef((PyObject *)self); + PyObject *args[] = { NULL, PyLong_FromLong(maxsize), }; if (args[1] == NULL) { return NULL; diff --git a/Objects/cownobject.c b/Objects/cownobject.c new file mode 100644 index 000000000000000..ada9239995e35b3 --- /dev/null +++ b/Objects/cownobject.c @@ -0,0 +1,587 @@ +#include "Python.h" +#include "pymacro.h" + +#include "pycore_cown.h" +#include "pycore_immutability.h" +#include "pycore_lock.h" +#include "pycore_time.h" // _PyTime_FromSeconds() + +/* 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); } + +#define Region_Check(x) Py_IS_TYPE((x), &_PyTracingRegion_Type) + +// The interpreter id 0 is used. This value will be used to indicate that +// no interpreter owns the cown. +#define RELEASED_IPID ((_PyCown_ipid_t)0xff00ff00ff00ff00LL) +#define GC_IPID ((_PyCown_ipid_t)0xffff00ff00ff00ffLL) +#define NO_BLOCKING_TIMEOUT -1 +#define UNSET_THREAD_ID ((_PyCown_ipid_t)0xff00000000000000LL) + +typedef enum CownLockStatus { + COWN_ACQUIRE_ERROR = -1, + COWN_ACQUIRE_FAIL = 0, + COWN_ACQUIRE_SUCCESS = 1 +} CownLockStatus; + +// Cowns rely on the immutability machinery for atomic reference counting: +// PyCown_init() freezes each instance once its initial value is installed. +struct _PyCownObject { + PyObject_HEAD + /* The id of the interpreter that currently owns this cown. + * + * This value may be read from and written to from different threads. + * Only use atomic operations to access this field. + */ + // FIXME(cowns): xFrednet: Make sure that an interpreter releases all + // cowns on destruction. + _PyCown_ipid_t owning_ip; + + /* The id of the thread that unlocked this cown. + * + * This is provided as additional information to users, it is not validated + * or used by this cown implementation. + */ + _PyCown_thread_id_t locking_thread; + + /* The value stored in the cown. This value may be immutable, another cown + * or a region object. + */ + PyObject* value; + + /* A lock used, mainly to support timeouts and queueing for locking. + * All other functions should use `owning_ip` to determine if they can + * access the data or not. + * + * Python's mutexes already implement queueing and timeouts in a good way. + * Later we can role our own, if we need but for not this is better. Note + * that the optional GIL release from the lock should not be used, as it + * doesn't seem to account for waiting threads from different interpreters. + * Therefore, we are responsible for releasing and acquireing the GIL. + */ + PyMutex lock; +}; + +static _PyCown_ipid_t cown_get_owner(_PyCownObject *obj) { + return _Py_atomic_load_uint64(&obj->owning_ip); +} + +#define BAIL_UNLESS_OWNED_BY(o, owned_by, result) \ + do {\ + _PyCown_ipid_t owning_ip = cown_get_owner(_PyCownObject_CAST(o)); \ + if (owning_ip != owned_by) { \ + PyErr_Format( \ + PyExc_RuntimeError, \ + "attempted to access a cown owned by %llu from %llu", \ + owning_ip, owned_by); \ + return result; \ + } \ + } while (0); +#define BAIL_UNLESS_OWNED(o, result) BAIL_UNLESS_OWNED_BY(o, _PyCown_ThisInterpreterId(), result) +#define BAIL_UNLESS_OWNED_NULL(o) BAIL_UNLESS_OWNED(o, NULL) + +static int cown_set_value_unchecked(_PyCownObject* self, PyObject* value) { + // Update the value + Py_XSETREF(self->value, Py_NewRef(value)); + + return 0; +} + +static int cown_set_value(_PyCownObject* self, PyObject* value) { + BAIL_UNLESS_OWNED(self, -1); + + // Bridge objects are allowed + if (Region_Check(value)) { + return cown_set_value_unchecked(self, value); + } + + // Immutable objects are allowed + if (_Py_IsImmutable(value)) { + return cown_set_value_unchecked(self, value); + } + + // Local objects are forbidden + PyErr_Format( + PyExc_RuntimeError, + "attempted to store a local mutable object in a cown.\n" + "Only regions, cown, and immutable objects are allowed"); + + return -1; +} + +/* Attempt to lock the cown. + * + * Timeout values: + * (-1) => Non-blocking locking + * (0) => Block with no timeout + * (n) => Blocking with timeout + */ +static int cown_lock(_PyCownObject* self, PyTime_t timeout, _PyCown_ipid_t locking_ip, bool has_gil) { + // A blocking time should only be set, if this call holds the GIL + assert(has_gil || timeout == NO_BLOCKING_TIMEOUT); + + // Try to lock the mutex directly, without releasing the GIL first + PyLockStatus r = _PyMutex_LockTimed(&self->lock, 0, _Py_LOCK_DONT_DETACH); + + // The cown is currently owned by something else. Release the GIL and + // wait for the timeout. + if (r != PY_LOCK_ACQUIRED && timeout != NO_BLOCKING_TIMEOUT) { + // Release the GIL + Py_BEGIN_ALLOW_THREADS; + + // Attempt to lock the mutex. This uses a PyMutex for the locking, + // timeout and signal handling. + r = _PyMutex_LockTimed( + &self->lock, + timeout, + _Py_LOCK_DONT_DETACH | _PY_LOCK_HANDLE_SIGNALS + ); + + // Acquire the GIL + Py_END_ALLOW_THREADS; + } + + // The lock was interrupted + if (r == PY_LOCK_INTR) { + return COWN_ACQUIRE_ERROR; + } + + // The lock acquisition failed + if (r == PY_LOCK_FAILURE) { + return COWN_ACQUIRE_FAIL; + } + + // Set the owning_ip to the current interpreter, thereby taking ownership + _PyCown_ipid_t released_value = RELEASED_IPID; + if (!_Py_atomic_compare_exchange_uint64( + &self->owning_ip, + &released_value, + locking_ip) + ) { + // Failed to set owning_ip, this should never happen and points + // to a deeper issue. + PyErr_Format( + PyExc_RuntimeError, + "[BUG] failed to set owner on a locked cown\n" + "Cown: %U", + self + ); + + _PyMutex_Unlock(&self->lock); + return COWN_ACQUIRE_ERROR; + } + + // Set the locking thread. + if (has_gil) { + self->locking_thread = _PyCown_ThisThreadId(); + } else { + self->locking_thread = UNSET_THREAD_ID; + } + + if (self->value && Region_Check(self->value)) { + assert(!PyObject_GC_IsTracked(self->value)); + PyObject_GC_Track(self->value); + } + + return COWN_ACQUIRE_SUCCESS; +} + +/* Returns the interpreter id used by cowns. + * + * The caller must hold the GIL. + */ +_PyCown_ipid_t _PyCown_ThisInterpreterId(void) { + _PyCown_ipid_t ip = PyInterpreterState_GetID(PyInterpreterState_Get()); + // This should never happen... if it does... we have a problem... + assert(ip != RELEASED_IPID); + return ip; +} + +/* Returns the thread id used by cowns. + * + * The caller must hold the GIL. + */ +_PyCown_thread_id_t _PyCown_ThisThreadId(void) { + _PyCown_thread_id_t id = PyThreadState_GetID(PyThreadState_Get()); + return id; +} + +static int PyCown_init(_PyCownObject *self, PyObject *args, PyObject *kwds) { + // See if we got a value as a keyword argument + static char *kwlist[] = {"value", NULL}; + PyObject *value = Py_None; + if (!PyArg_ParseTupleAndKeywords(args, kwds, "|O", kwlist, &value)) { + return -1; + } + + // Init the cown as being acquired by the current interpreter + _PyCown_ipid_t this_ip = _PyCown_ThisInterpreterId(); + _Py_atomic_store_uint64(&self->owning_ip, RELEASED_IPID); + if (cown_lock(self, NO_BLOCKING_TIMEOUT, this_ip, true) != COWN_ACQUIRE_SUCCESS) { + PyErr_Format( + PyExc_RuntimeError, + "Newly created cown couldn't be acquired by interpreter %lld (this)", + this_ip); + return -1; + } + + // Set the cown value using the internal function for full validation + SUCCEEDS(cown_set_value(self, value)); + + // Freeze the cown to enable atomic reference counting for it. + PyObject_GC_UnTrack(self); + SUCCEEDS(_PyImmutability_Freeze(_PyObject_CAST(self))); + + return 0; +error: + return -1; +} + +static int PyCown_traverse(_PyCownObject *self, visitproc _ignore1, void* _ignore2) { + // tp_traverse should never be called on cowns since they're not + // tracked by the GC or in any other GC list. The cown type + // still defines `tp_traverse` to ensure that this is never + // accidentally called. Later we may want to simple remove it + // from the type. + assert(false); + return -1; +} + +static int PyCown_reachable(_PyCownObject *self, visitproc visit, void *arg) { + Py_VISIT(Py_TYPE(self)); + + // The value is explicitly not visited. Freezing or moving cowns should + // not propagate to the value. + // Py_VISIT(self->value); + + return 0; +} + +static int PyCown_clear(_PyCownObject *self) { + cown_set_value_unchecked(self, Py_None); + Py_CLEAR(self->value); + return 0; +} + +static void PyCown_dealloc(_PyCownObject *self) { + PyObject_GC_UnTrack(self); + PyCown_clear(self); + PyObject_GC_Del(self); +} + +static int +lock_acquire_parse_args(PyObject *args, PyObject *kwds, + PyTime_t *timeout) +{ + // Taken from `Modules/_threadmodule.c` + + char *kwlist[] = {"blocking", "timeout", NULL}; + int blocking = 1; + PyObject *timeout_obj = NULL; + if (!PyArg_ParseTupleAndKeywords(args, kwds, "|pO:acquire", kwlist, + &blocking, &timeout_obj)) + return -1; + + const PyTime_t unset_timeout = _PyTime_FromSeconds(NO_BLOCKING_TIMEOUT); + *timeout = unset_timeout; + + if (timeout_obj + && _PyTime_FromSecondsObject(timeout, + timeout_obj, _PyTime_ROUND_TIMEOUT) < 0) + return -1; + + if (!blocking && *timeout != unset_timeout ) { + PyErr_SetString(PyExc_ValueError, + "can't specify a timeout for a non-blocking call"); + return -1; + } + if (*timeout < 0 && *timeout != unset_timeout) { + PyErr_SetString(PyExc_ValueError, + "timeout value must be a non-negative number"); + return -1; + } + if (!blocking) + *timeout = 0; + else if (*timeout != unset_timeout) { + PyTime_t microseconds; + + microseconds = _PyTime_AsMicroseconds(*timeout, _PyTime_ROUND_TIMEOUT); + if (microseconds > PY_TIMEOUT_MAX) { + PyErr_SetString(PyExc_OverflowError, + "timeout value is too large"); + return -1; + } + } + return 0; +} + +static PyObject * +CownObject_acquire(_PyCownObject *self, PyObject *args, PyObject *kwds) +{ + // Parse the arguments + PyTime_t timeout; + if (lock_acquire_parse_args(args, kwds, &timeout) < 0) { + return NULL; + } + + // Attempt to lock the cown + _PyCown_ipid_t this_ip = _PyCown_ThisInterpreterId(); + int res = cown_lock(self, timeout, this_ip, true); + if (res == COWN_ACQUIRE_ERROR) { + return NULL; + } + + // Return the result + return PyBool_FromLong(res == COWN_ACQUIRE_SUCCESS); +} + +PyDoc_STRVAR(CownObject_acquire_doc, +"acquire($self, /, blocking=True, timeout=-1)\n\ +--\n\ +\n\ +Attempts to acquires the cown. With default arguments this will block\n\ +until the cown can be aquired, even when acquire is called from the same\n\ +interpreter. The return indicates if the cown was\n\ +was acquired. The blocking operation is interruptible."); + +static int cown_release_unchecked(_PyCownObject* self, _PyCown_ipid_t unlocking_ip) { + // Set owning_ip to indicate the released state + if (!_Py_atomic_compare_exchange_uint64(&self->owning_ip, &unlocking_ip, RELEASED_IPID)) { + PyErr_Format( + PyExc_RuntimeError, + "interpreter %lld (this) attempted to release a cown owned by someone else\n" + "Cown: %U", + unlocking_ip, self); + return -1; + } + + // Unlocking should always succeed + int res = _PyMutex_TryUnlock(&self->lock); + assert(res == 0); + (void)res; + + return 0; +} + +/* Checks that the cown is not released, and that the owner is as the current interpreter. */ +static int cown_check_owner_before_release(_PyCownObject *self, _PyCown_ipid_t unlocking_ip) { + _PyCown_ipid_t owning_ip = cown_get_owner(self); + if (owning_ip == RELEASED_IPID) { + PyErr_Format( + PyExc_RuntimeError, + "interpreter %lld attempted to release/switch a released cown", + unlocking_ip + ); + return -1; + } + if (owning_ip != unlocking_ip) { + PyErr_Format( + PyExc_RuntimeError, + "interpreter %lld attempted to release/switch a cown owned by %lld", + unlocking_ip, owning_ip + ); + return -1; + } + return 0; +} + +/* This attempts to close the region + * + * It returns non-zero if the closing failed + */ +static int cown_close_region(_PyCownObject *self) { + assert(Region_Check(self->value)); + + // Close the region + int closing_res = _PyTracingRegion_Close(self->value); + if (closing_res < 0) { + return -1; + } + + // Make sure that the cown owns the only external reference to the bridge object. + if (Py_REFCNT(self->value) > 1) { + PyErr_Format( + PyExc_RuntimeError, + "the cown couldn't be released, due to the bridge having incoming references"); + return -1; + } + + // The region is closed and this is the only owner of the bridge. We untrack + // from the current GC list. + PyObject_GC_UnTrack(self->value); + + return 0; +} + +static int cown_release(_PyCownObject *self, _PyCown_ipid_t unlocking_ip) { + if (cown_check_owner_before_release(self, unlocking_ip) < 0) { + return -1; + } + + // Immutable objects are safe to share, the cown can be release directly + if (_Py_IsImmutable(self->value)) { + return cown_release_unchecked(self, unlocking_ip); + } + assert(Region_Check(self->value)); + + // The contained region needs to be closed, to allow the cown to release + if (cown_close_region(self)) { + return -1; + } + + // Region is closed, safe to release + return cown_release_unchecked(self, unlocking_ip); +} + +static PyObject* CownObject_release(_PyCownObject *self, PyObject *ignored) { + _PyCown_ipid_t this_ip = _PyCown_ThisInterpreterId(); + if (cown_release(self, this_ip) < 0) { + return NULL; + } + + Py_RETURN_NONE; +} + +PyDoc_STRVAR(CownObject_release_doc, +"release($self, /)\n\ +--\n\ +\n\ +Release the cown, allowing another interpreter that is blocked waiting for\n\ +the cown to acquire the cown. The cown must be in the locked state\n\ +and must be unlocked from the owning interpreter. It may be unlocked \n\ +by any thread on the owning interpreter."); + +static PyObject * +CownObject_locked(_PyCownObject *op, PyObject *Py_UNUSED(dummy)) +{ + return PyBool_FromLong(cown_get_owner(op) != RELEASED_IPID); +} + +PyDoc_STRVAR(CownObject_locked_doc, +"locked($self, /)\n\ +--\n\ +\n\ +Return whether the cown currently released or aquired. \n\ +Use `owned()` to check if the cown is aquired by the current interpreter."); + +static PyObject * +CownObject_owned(_PyCownObject *op, PyObject *Py_UNUSED(dummy)) +{ + return PyBool_FromLong(cown_get_owner(op) == _PyCown_ThisInterpreterId()); +} + +PyDoc_STRVAR(CownObject_owned_doc, +"owned($self, /)\n\ +--\n\ +\n\ +Return true if the cown is currently aquired by this interpreter, false otherwise."); + +static PyObject * +CownObject_owned_by_thread(_PyCownObject *op, PyObject *Py_UNUSED(dummy)) +{ + if (cown_get_owner(op) != _PyCown_ThisInterpreterId()) { + Py_RETURN_FALSE; + } + + return PyBool_FromLong(op->locking_thread == _PyCown_ThisThreadId()); +} + +PyDoc_STRVAR(CownObject_owned_by_thread_doc, +"owned($self, /)\n\ +--\n\ +\n\ +Return true if the cown is currently aquired by this interpreter and was \n\ +locked by the current thread, false otherwise. \n\ +Ownership on the thread level is not enforced, any thread on the owning\n\ +interpreter can access and release the cown. This is information is only\n\ +provided to give more control for those who seek it."); + +static PyObject * +CownObject_is_closed(_PyCownObject *self, PyObject *Py_UNUSED(dummy)) +{ + if (!Region_Check(self->value)) { + PyErr_SetString(PyExc_TypeError, "cown value is not a tracing region"); + return NULL; + } + + return PyBool_FromLong(_PyTracingRegion_IsClosed(self->value)); +} + +PyDoc_STRVAR(CownObject_is_closed_doc, +"_is_closed($self, /)\n\ +--\n\ +\n\ +Return true if the cown's tracing region value is closed."); + + +// Define the CownType with methods +static PyMethodDef PyCown_methods[] = { + {"acquire", _PyCFunction_CAST(CownObject_acquire), METH_VARARGS | METH_KEYWORDS, CownObject_acquire_doc}, + {"release", _PyCFunction_CAST(CownObject_release), METH_NOARGS, CownObject_release_doc}, + {"locked", _PyCFunction_CAST(CownObject_locked), METH_NOARGS, CownObject_locked_doc}, + {"owned", _PyCFunction_CAST(CownObject_owned), METH_NOARGS, CownObject_owned_doc}, + {"owned_by_thread", _PyCFunction_CAST(CownObject_owned_by_thread), METH_NOARGS, CownObject_owned_by_thread_doc}, + {"_is_closed", _PyCFunction_CAST(CownObject_is_closed), METH_NOARGS, CownObject_is_closed_doc}, + {NULL} // Sentinel +}; + +static PyObject *CownObject_get_value(_PyCownObject *self, void *closure) { + BAIL_UNLESS_OWNED_NULL(self); + + return Py_NewRef(self->value); +} + +static int CownObject_set_value(_PyCownObject *self, PyObject *value, void *closure) { + BAIL_UNLESS_OWNED(self, -1); + + return cown_set_value(self, value); +} + +static PyGetSetDef PyCownObject_getset[] = { + {"value", (getter)CownObject_get_value, (setter)CownObject_set_value, + "", NULL}, + {NULL, NULL, NULL, NULL, NULL} +}; + +static PyObject *PyCown_repr(_PyCownObject *self) { + _PyCown_ipid_t owner = cown_get_owner(self); + // On this interpreter we can access the cown and content + // safely since we hold the GIL + if (owner == _PyCown_ThisInterpreterId()) { + return PyUnicode_FromFormat( + "Cown(interpreter=%llu (this), value=%S)", + owner, + PyObject_Repr(self->value) + ); + } + + // The cown is released and can be acquired + if (owner == RELEASED_IPID) { + return PyUnicode_FromFormat( + "Cown(interpreter=None, status=Released)" + ); + } + + // The cown is owned by a different interpreter + return PyUnicode_FromFormat( + "Cown(interpreter=%llu (other))", + owner + ); +} + +PyTypeObject _PyCown_Type = { + PyVarObject_HEAD_INIT(&PyType_Type, 0) + .tp_name = "Cown", + .tp_basicsize = sizeof(_PyCownObject), + .tp_dealloc = (destructor)PyCown_dealloc, + .tp_repr = (reprfunc)PyCown_repr, + .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_IMMUTABLETYPE, + .tp_traverse = (traverseproc)PyCown_traverse, + .tp_reachable = (traverseproc)PyCown_reachable, + .tp_clear = (inquiry)PyCown_clear, + .tp_methods = PyCown_methods, + .tp_getset = PyCownObject_getset, + .tp_init = (initproc)PyCown_init, + .tp_new = PyType_GenericNew, +}; + diff --git a/Objects/tracingregionobject.c b/Objects/tracingregionobject.c new file mode 100644 index 000000000000000..6cc182ce84ef4ce --- /dev/null +++ b/Objects/tracingregionobject.c @@ -0,0 +1,1760 @@ +#include "Python.h" +#include "pycore_interp.h" +#include "pycore_gc.h" // _PyObject_GC_IS_TRACKED() +#include "pycore_dict.h" // _PyObject_MaterializeManagedDict() +#include "pycore_object.h" // _PyObject_GC_TRACK(), _PyDebugAllocatorStats() +#include "pycore_descrobject.h" +#include "pycore_modsupport.h" // _PyArg_NoPositional() +#include "pycore_weakref.h" +#include "pycore_cown.h" + +#define ERROR_OBJECT_REPORT_COUNT 5 +#define ERROR_MERMAID_REPORT_LIMIT 50 +#define ERROR_MERMAID_HIDE_IMMUTABLE true + +/* Set this to the path of the file that a failed close should write its mermaid + * graph to. The graph is not written when the variable is unset or empty. */ +#define REGION_GRAPH_ENV_VAR "PYTHON_REGION_GRAPH" + +#define REGION_TRACING + +#ifdef REGION_TRACING +#define dbg(msg, ...) \ + do { \ + printf(msg "\n" __VA_OPT__(,) __VA_ARGS__); \ + } while(0) +#else +#define dbg(...) +#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) + +#define Region_Check(x) Py_IS_TYPE((x), &_PyTracingRegion_Type) +#define Cown_Check(x) Py_IS_TYPE((x), &_PyCown_Type) + +// ################################################################### +// Copied from gc.c +// ################################################################### + +#ifndef Py_GIL_DISABLED +#define GC_NEXT _PyGCHead_NEXT +#define GC_PREV _PyGCHead_PREV + +static inline int +gc_old_space(PyGC_Head *g) +{ + return g->_gc_next & _PyGC_NEXT_MASK_OLD_SPACE_1; +} + +static inline void +gc_set_old_space(PyGC_Head *g, int space) +{ + assert(space == 0 || space == _PyGC_NEXT_MASK_OLD_SPACE_1); + g->_gc_next &= ~_PyGC_NEXT_MASK_OLD_SPACE_1; + g->_gc_next |= space; +} + +static inline void +gc_list_init(PyGC_Head *list) +{ + // List header must not have flags. + // We can assign pointer by simple cast. + list->_gc_prev = (uintptr_t)list; + list->_gc_next = (uintptr_t)list; +} + +static void +gc_list_move(PyGC_Head *node, PyGC_Head *list) +{ + /* Unlink from current list. */ + PyGC_Head *from_prev = GC_PREV(node); + PyGC_Head *from_next = GC_NEXT(node); + _PyGCHead_SET_NEXT(from_prev, from_next); + _PyGCHead_SET_PREV(from_next, from_prev); + + /* Relink at end of new list. */ + // list must not have flags. So we can skip macros. + PyGC_Head *to_prev = (PyGC_Head*)list->_gc_prev; + _PyGCHead_SET_PREV(node, to_prev); + _PyGCHead_SET_NEXT(to_prev, node); + list->_gc_prev = (uintptr_t)node; + _PyGCHead_SET_NEXT(node, list); +} + +static inline int +gc_list_is_empty(PyGC_Head *list) +{ + return (list->_gc_next == (uintptr_t)list); +} + +static void +gc_list_merge(PyGC_Head *from, PyGC_Head *to) +{ + assert(from != to); + if (!gc_list_is_empty(from)) { + PyGC_Head *to_tail = GC_PREV(to); + PyGC_Head *from_head = GC_NEXT(from); + PyGC_Head *from_tail = GC_PREV(from); + assert(from_head != from); + assert(from_tail != from); + assert(gc_list_is_empty(to) || + gc_old_space(to_tail) == gc_old_space(from_tail)); + + _PyGCHead_SET_NEXT(to_tail, from_head); + _PyGCHead_SET_PREV(from_head, to_tail); + + _PyGCHead_SET_NEXT(from_tail, to); + _PyGCHead_SET_PREV(to, from_tail); + } + gc_list_init(from); +} + +static struct _gc_runtime_state* +get_gc_state(void) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + return &interp->gc; +} + +static inline void +gc_clear_collecting(PyGC_Head *g) +{ + g->_gc_prev &= ~_PyGC_PREV_MASK_COLLECTING; +} + +#else // Py_GIL_DISABLED +#error "We need GIL" +#endif + +// ################################################################### +// Copied from regions-main +// ################################################################### + +/* Removes the last item of the list and returns it as a new reference. + * + * The caller needs a reference of its own, since the list was the only thing + * keeping the item alive. Traversing the item can run arbitrary code, for + * example through `_PyImmutability_Freeze()`, which could otherwise deallocate + * it while it is being traversed. + * + * Returns NULL with an exception set on failure. The list must not be empty. + */ +static PyObject* list_pop(PyObject* s){ + Py_ssize_t size = PyList_GET_SIZE(s); + assert(size > 0); + + PyObject *item = Py_NewRef(PyList_GET_ITEM(s, size - 1)); + // This should never fail, since we shrink the size + if (PyList_SetSlice(s, size - 1, size, NULL)) { + Py_DECREF(item); + return NULL; + } + return item; +} + +typedef enum { + Py_MOVABLE_YES = 0, + Py_MOVABLE_NO = 1, + // The object should be frozen + Py_MOVABLE_FREEZE = 2, + // The object is not movable, but the reference is allowed. The object + // should be skipped + Py_MOVABLE_COWN = 3, +} movable_status; + +static movable_status get_movable_status(PyObject *obj) { + // FIXME(regions): xFrednet: Currently it's not possible to set + // the movability per object. This instead returns the default + // movability for objects. Note that some shallow immutable objects + // will not return freeze as their movability. + + // Immortal object have no real RC, this makes it infeasible to have them + // in a region and dynamically track their ownership. Immortal objects are + // intended to be immutable in Python, so it should be safe to implicitly + // freeze them. + if (_Py_IsImmortal(obj)) { + return Py_MOVABLE_FREEZE; + } + + // Immutable objects don't need to be moved + if (_Py_IsImmutable(obj)) { + return Py_MOVABLE_FREEZE; + } + + // Types are a pain for regions since it's likely that objects of one type may + // end up in multiple regions, requiring the type to be frozen. Types also + // have a lot of reference pointing to them. Let's hope there is no need to + // keep them freezable + if (PyType_Check(obj)) { + return Py_MOVABLE_FREEZE; + } + + // Module objects are also complicated. Freezing them should turn most modules + // into proxies which should make them mostly usable. + if (PyModule_Check(obj)) { + return Py_MOVABLE_FREEZE; + } + + // Functions are a mess as well, making the entire system reachable. Freezing + // them should again just magically make most things work + if (PyFunction_Check(obj)) { + return Py_MOVABLE_FREEZE; + } + + // CWrappers can't really be owned, but need some special handling since + // interpreters could still race on their RC. Solution, throw them in the + // freezer + if (PyCFunction_Check(obj) + || Py_IS_TYPE(obj, &_PyMethodWrapper_Type) + || Py_IS_TYPE(obj, &PyWrapperDescr_Type) + ) { + return Py_MOVABLE_FREEZE; + } + + // Cowns are not movable, but the reference is explicitly allowed. + if (Cown_Check(obj)) { + return Py_MOVABLE_COWN; + } + + // Freezing or moving these objects is... complicated. In some cases it is + // possible but more hassle than it's probably worth. For now we mark them + // all as unmovable. + if (PyFrame_Check(obj) + || PyGen_CheckExact(obj) + || PyCoro_CheckExact(obj) + || PyAsyncGen_CheckExact(obj) + || PyAsyncGenASend_CheckExact(obj) + ) { + return Py_MOVABLE_NO; + } + + // Exceptions don't hold anything obviously problematic preventing them + // from being moved into a region. The actual problem is that the runtime + // stores references to them and that these are already emitted on an + // error path. Moving them into a region could add more problems. + // We should discuss how to handle these, maybe freezing is the correct + // approach? + if (PyExceptionInstance_Check(obj)) { + return Py_MOVABLE_NO; + } + + // Regions are theoretically only movable, if they're closed. The traversal + // checks this manually. + + // For now, we define all other objects as movable by default. (Surely + // this will not backfire) + return Py_MOVABLE_YES; +} + +// This uses the given arguments to create and throw a `RuntimeError` +static void throw_region_error( + const char *format_str, const char *tp_name, + PyObject* src, PyObject* tgt) +{ + // Don't stomp existing exception + PyThreadState *tstate = PyThreadState_Get(); + if (_PyErr_Occurred(tstate)) { + return; + } + + PyErr_Format(PyExc_RuntimeError, format_str, tp_name); + + PyObject *exc = PyErr_GetRaisedException(); + assert(exc != NULL); + + // Failing to attach it must not replace the error raised above. + if (PyObject_SetAttr(exc, &_Py_ID(source), src ? src : Py_None) < 0 + || PyObject_SetAttr(exc, &_Py_ID(target), tgt ? tgt : Py_None) < 0) + { + PyErr_Clear(); + } + + PyErr_SetRaisedException(exc); +} + +// Wrapper around tp_traverse that also visits the type object. +static int +traverse_via_tp_traverse(PyObject *obj, visitproc visit, void *state) +{ + PyTypeObject *tp = Py_TYPE(obj); + + // Visit the type with traverse + traverseproc traverse = tp->tp_traverse; + if (traverse != NULL) { + int err = traverse(obj, visit, state); + if (err) { + return err; + } + } + + // Most `tp_traverse` don't visit the type even though they should. + // Here it won't hurt to potentially visit it twice, since types + // are non-movable but will be frozen. + return visit((PyObject *)tp, state); +} + +/* Returns the appropriate traversal function for reaching all references from + * an object. Prefers tp_reachable, falls back to tp_traverse wrapped to also + * visit the type. + * + * Falling back means the trace can miss references that only tp_reachable + * reports, so every type it happens for is recorded in `missing_reachable` and + * reported by `report_missing_reachable()` once the trace is over. Warning here + * would write to `sys.stderr` in the middle of the traversal, which can run + * arbitrary Python code and invalidate the reference counts already sampled. + * + * `missing_reachable` may be NULL to skip the recording. + */ +static traverseproc +get_reachable_proc(PyTypeObject *tp, _Py_hashtable_t *missing_reachable) +{ + if (tp->tp_reachable != NULL) { + return tp->tp_reachable; + } + + if (missing_reachable != NULL + && _Py_hashtable_get_entry(missing_reachable, tp) == NULL) + { + // Types are frozen rather than moved, so `_move_obj()` returns before it + // samples their reference count. Holding one here can therefore not + // disturb the LRC of any region. + if (_Py_hashtable_set(missing_reachable, Py_NewRef(tp), + (void *)(Py_uintptr_t)(tp->tp_traverse != NULL)) < 0) { + Py_DECREF(tp); + // A failed warning must not fail the close. + PyErr_Clear(); + } + } + + // Always return the wrapper; even when tp_traverse is NULL, the wrapper + // will still visit the type object which tp_reachable is expected to do. + return traverse_via_tp_traverse; +} + +static int +report_missing_reachable_type( + _Py_hashtable_t *ht, const void *key, const void *value, void *user_data) +{ + PyTypeObject *tp = (PyTypeObject *)key; + if (value) { + PySys_FormatStderr( + "regions: type '%.100s' has tp_traverse but no tp_reachable\n", + tp->tp_name); + } + else { + PySys_FormatStderr( + "regions: type '%.100s' has no tp_traverse and no tp_reachable\n", + tp->tp_name); + } + return 0; +} + +static int +release_missing_reachable_type( + _Py_hashtable_t *ht, const void *key, const void *value, void *user_data) +{ + Py_DECREF((PyObject *)key); + return 0; +} + +// ################################################################### +// Tracing Impl +// ################################################################### + +static void +gc_list_dissolve(PyGC_Head *list) { + struct _gc_runtime_state* gc_state = get_gc_state(); + gc_list_merge(list, &(gc_state->old[0].head)); +} + +static int +detach_weak_refs_visit(_Py_hashtable_t *ht, const void *key, const void *value, void *user_data) +{ + PyObject *item = (PyObject *)key; + if (!_PyType_SUPPORTS_WEAKREFS(Py_TYPE(item))) { + return 0; + } + +#ifdef Py_DEBUG + Py_ssize_t weak_ctn = _PyWeakref_GetWeakrefCount(item); + if (weak_ctn) { + dbg("- Clearing %zd weak references to %p", weak_ctn, item); + } +#endif + _PyWeakref_ClearWeakRefsNoCallbacks(item); + return 0; +} + +/* Detaches all weak references pointing to objects inside the region. + * + * This walks the set of traced objects instead of the region's GC list, since + * objects that are not tracked by the GC never enter that list. Missing one + * would leave a live weak reference pointing into the closed region, which is + * enough for external code to read and mutate its contents. + */ +static void detach_weak_refs(_Py_hashtable_t *visited) { + // `detach_weak_refs_visit()` never fails, so the result can be ignored. + (void)_Py_hashtable_foreach(visited, detach_weak_refs_visit, NULL); +} + +typedef struct { + PyObject_HEAD + PyObject *dict; + // The GC list containing all objects while the region is closed. The bridge + // object is not in this GC list but in the list of the owning region or in no + // list if it's owned by a released cown. + PyGC_Head gc_list; + // FIXME(regions): This can be inferred from the status of the gc_list + // or stored in the lower bits of the GC list. For now we keep it separate + // for the prototype + bool open; + // This is the number of references from inside the region that reference + // this bridge object. + Py_ssize_t internal_bridge_refs; +} TracingRegionObject; + +static void _region_close( + TracingRegionObject *self, + Py_ssize_t bridge_rc, + _Py_hashtable_t *visited +) { + if (!self->open) { + return; + } + + dbg("Closing region %p", self); + + detach_weak_refs(visited); + + // TODO(regions): explain RC magic + if (bridge_rc != 0) { + assert(bridge_rc >= 0); + dbg("- subtracting %zd internal references from the bridge object %p", bridge_rc, self); + _Py_RefcntAdd(self, -bridge_rc); + self->internal_bridge_refs = bridge_rc; + } else { + assert(self->internal_bridge_refs == 0); + } + + self->open = false; +} + +/* Re-adds the references to the bridge object that `_region_close()` subtracted. + * + * Note that this may resurrect the bridge object. Callers may need to handle this case. + */ +static void _restore_internal_bridge_refs(TracingRegionObject *self) { + if (self->internal_bridge_refs != 0) { + assert(self->internal_bridge_refs >= 0); + dbg("- adding %zd internal references from the bridge object %p", self->internal_bridge_refs, self); + _Py_RefcntAdd(self, self->internal_bridge_refs); + self->internal_bridge_refs = 0; + } +} + +static void _open_region(TracingRegionObject *self) { + if (self->open) { + return; + } + + dbg("Opening region %p", self); + + _restore_internal_bridge_refs(self); + + // This only dissolves this region, all sub-regions remain closed. + gc_list_dissolve(&self->gc_list); + assert(gc_list_is_empty(&self->gc_list)); + + self->open = true; +} + +#define PER_REGION_TRACE_LIMIT 2 + +typedef struct { + // This is the stack of regions that still need to be closed to close this + // region tree. A region stays on the stack until it is closed, so anything + // its trace discovers is pushed on top of it and handled first. The loop can + // therefore only drain once every region in the tree is closed. + // + // How many attempts a region gets is tracked by `tracing_counts`. + PyObject *pending; + // This tracks per region in the tree how often it has been traversed. + // Some things require the trace to be redone, namely freezing an object + // as that may create references and finding an open sub-region, as that + // one needs to be traced and closed first. + // + // We limit the number of times we restart the trace per region. + // Theoretically, this may reject some programs that would eventually + // reach a fixed point, but if somebody wants to do dark magic, that's + // really not our problem. + _Py_hashtable_t *tracing_counts; + // The types that had to be traversed via tp_traverse because they have no + // tp_reachable. Used to report each of them once per trace, see + // `get_reachable_proc()`. + _Py_hashtable_t *missing_reachable; +} tree_trace_state_t; + +static void tree_trace_state_destroy(tree_trace_state_t* state) { + if (state->tracing_counts) { + _Py_hashtable_destroy(state->tracing_counts); + state->tracing_counts = NULL; + } + if (state->missing_reachable) { + (void)_Py_hashtable_foreach( + state->missing_reachable, release_missing_reachable_type, NULL); + _Py_hashtable_destroy(state->missing_reachable); + state->missing_reachable = NULL; + } + if (state->pending) { + Py_CLEAR(state->pending); + } +} + +/* Reports the types that `get_reachable_proc()` had to fall back for. + * + * This has to run after the traversal is over, since writing to `sys.stderr` + * can execute arbitrary Python code. + */ +static void report_missing_reachable(tree_trace_state_t* state) { + if (state->missing_reachable == NULL + || _Py_hashtable_len(state->missing_reachable) == 0) + { + return; + } + + // Keep whatever the trace is raising; a failed warning is not worth + // replacing a region error with. + PyObject *exc = PyErr_GetRaisedException(); + (void)_Py_hashtable_foreach( + state->missing_reachable, report_missing_reachable_type, NULL); + PyErr_SetRaisedException(exc); +} + +static int tree_trace_state_init(tree_trace_state_t* state) { + // Both fields have to be cleared up front, so that the error path below can + // call `tree_trace_state_destroy()` before they have all been assigned. + state->tracing_counts = NULL; + state->missing_reachable = NULL; + state->pending = NULL; + + state->tracing_counts = _Py_hashtable_new( + _Py_hashtable_hash_ptr, + _Py_hashtable_compare_direct); + if (state->tracing_counts == NULL) { + goto error; + } + + state->missing_reachable = _Py_hashtable_new( + _Py_hashtable_hash_ptr, + _Py_hashtable_compare_direct); + if (state->missing_reachable == NULL) { + goto error; + } + + state->pending = PyList_New(0); + if (state->pending == NULL) { + goto error; + } + + return 0; +error: + tree_trace_state_destroy(state); + return -1; +} + +typedef struct { + // List of pending objects that are not GC + PyObject *pending; + // A list of all visited objects + _Py_hashtable_t *visited; + + // The trace state belonging to the region tree that this region + // is a part of. + tree_trace_state_t *tree_trace_state; + // The bridge object of the region that is currently being traced. + PyObject* bridge; + // The source of the reference, this is used for error reporting + PyObject *src; + + // The number of refs coming into this object graph + Py_ssize_t external_rc; + // The number of refs coming from inside the region to the bridge object + Py_ssize_t bridge_rc; + + // The GC list used for this trace, it may be null if the trace + // should not move the objects from their current list. + PyGC_Head* gc_list; + + + // This is set if an object was frozen and the trace needs + // to restart to be valid + bool restart; +} region_trace_state_t; + +static void region_trace_state_destroy(region_trace_state_t* state) { + if (state->pending) { + Py_CLEAR(state->pending); + } + if (state->visited) { + _Py_hashtable_destroy(state->visited); + state->visited = NULL; + } +} + +static int region_trace_state_init( + region_trace_state_t* state, + PyObject* bridge, + PyGC_Head* gc_list, + tree_trace_state_t *tree_trace_state +) { + assert(gc_list == NULL || gc_list_is_empty(gc_list)); + + state->pending = NULL; + state->visited = NULL; + + state->pending = PyList_New(0); + if (state->pending == NULL) { + goto error; + } + + state->visited = _Py_hashtable_new( + _Py_hashtable_hash_ptr, + _Py_hashtable_compare_direct); + if (state->visited == NULL) { + goto error; + } + + state->tree_trace_state = tree_trace_state; + state->bridge = bridge; + state->src = NULL; + + state->external_rc = 0; + state->bridge_rc = 0; + state->gc_list = gc_list; + state->restart = false; + + return 0; +error: + region_trace_state_destroy(state); + return -1; +} + +static void region_trace_state_set_restart(region_trace_state_t* state) { + state->restart = true; + // Setting the gc_list to NULL will stop objects from being moved + // between GC lists. Just a small thing we can avoid. The next (full) + // trace will have this set again. + state->gc_list = NULL; +} + +typedef struct { + // Every object with incoming references, used to mark up the mermaid graph. + _Py_hashtable_t *problem_objs; + // The subset of `problem_objs` that the error message lists, capped at + // `ERROR_OBJECT_REPORT_COUNT` entries. + _Py_hashtable_t *reported_objs; + Py_ssize_t incoming_refs; +} close_error_info_t; + +typedef struct { + _Py_hashtable_t *problem_objs; + _Py_hashtable_t *reported_objs; +} close_error_filter_t; + +typedef struct { + // A strong reference, see `collect_incoming_ref()`. + PyObject *obj; + Py_ssize_t refs; +} incoming_ref_entry_t; + +typedef struct { + // `collect_close_error_obj()` caps the reported set at this size. + incoming_ref_entry_t entries[ERROR_OBJECT_REPORT_COUNT]; + Py_ssize_t count; +} incoming_ref_report_t; + +typedef struct { + PyUnicodeWriter *writer; + _Py_hashtable_t *visited; + _Py_hashtable_t *problem_objs; + _Py_hashtable_t *reported_objs; + PyObject *pending; + PyObject *src; +} mermaid_dump_state_t; + +enum { + TRACE_RES_ERR = -1, + TRACE_RES_DONE = 0, + // The trace itself succeeded, but it was based on information that changed + // while it ran, so the region is still open and needs another attempt. + TRACE_RES_RESTART = 1, +}; + +static int +collect_close_error_obj(_Py_hashtable_t *ht, const void *key, const void *value, void *user_data) +{ + close_error_filter_t *filter = (close_error_filter_t *)user_data; + Py_ssize_t refs = (Py_ssize_t)value; + + // Objects whose every reference came from inside the region are not part of + // the problem. + if (refs <= 0) { + return 0; + } + if (_Py_hashtable_set(filter->problem_objs, key, (void *)refs) < 0) { + return -1; + } + if (_Py_hashtable_len(filter->reported_objs) < ERROR_OBJECT_REPORT_COUNT) { + if (_Py_hashtable_set(filter->reported_objs, key, (void *)refs) < 0) { + return -1; + } + } + return 0; +} + +static void +close_error_info_destroy(close_error_info_t *info) +{ + if (info->problem_objs != NULL) { + _Py_hashtable_destroy(info->problem_objs); + info->problem_objs = NULL; + } + if (info->reported_objs != NULL) { + _Py_hashtable_destroy(info->reported_objs); + info->reported_objs = NULL; + } +} + +static int +close_error_info_init(close_error_info_t *info, region_trace_state_t *state) +{ + info->incoming_refs = state->external_rc; + info->problem_objs = NULL; + info->reported_objs = NULL; + info->problem_objs = _Py_hashtable_new( + _Py_hashtable_hash_ptr, + _Py_hashtable_compare_direct); + if (info->problem_objs == NULL) { + return -1; + } + info->reported_objs = _Py_hashtable_new( + _Py_hashtable_hash_ptr, + _Py_hashtable_compare_direct); + if (info->reported_objs == NULL) { + close_error_info_destroy(info); + return -1; + } + + close_error_filter_t filter = {info->problem_objs, info->reported_objs}; + int res = _Py_hashtable_foreach(state->visited, collect_close_error_obj, &filter); + if (res < 0) { + close_error_info_destroy(info); + return -1; + } + return 0; +} + +static int +collect_incoming_ref(_Py_hashtable_t *ht, const void *key, const void *value, void *user_data) +{ + incoming_ref_report_t *report = (incoming_ref_report_t *)user_data; + + assert(report->count < ERROR_OBJECT_REPORT_COUNT); + if (report->count >= ERROR_OBJECT_REPORT_COUNT) { + return 0; + } + + incoming_ref_entry_t *entry = &report->entries[report->count]; + // The hashtable stores raw pointers without owning a reference. Taking one + // here keeps every reported object alive while `__str__` runs on the others, + // since that can execute arbitrary code and drop the last reference to any + // of them. + entry->obj = Py_NewRef((PyObject *)key); + entry->refs = (Py_ssize_t)value; + report->count += 1; + return 0; +} + +static void +incoming_ref_report_clear(incoming_ref_report_t *report) +{ + for (Py_ssize_t i = 0; i < report->count; i++) { + Py_CLEAR(report->entries[i].obj); + } + report->count = 0; +} + +static PyObject * +build_close_error_message(close_error_info_t *info) +{ + incoming_ref_report_t report = {{{NULL, 0}}, 0}; + PyUnicodeWriter *writer = NULL; + + // Collect the reported objects, and with them their references, before any + // of them is formatted below. + if (_Py_hashtable_foreach(info->reported_objs, collect_incoming_ref, &report) < 0) { + goto error; + } + + writer = PyUnicodeWriter_Create(0); + if (writer == NULL) { + goto error; + } + + if (PyUnicodeWriter_WriteUTF8(writer, + "The region could not be closed due to:\n", -1) < 0) { + goto error; + } + + Py_ssize_t accounted = 0; + for (Py_ssize_t i = 0; i < report.count; i++) { + PyObject *obj = report.entries[i].obj; + Py_ssize_t refs = report.entries[i].refs; + accounted += refs; + + if (PyUnicodeWriter_Format(writer, + "- %zd incoming reference%s to %s '%S'\n", + refs, (refs == 1) ? "" : "s", Py_TYPE(obj)->tp_name, obj) < 0) { + goto error; + } + } + + if (accounted < info->incoming_refs) { + Py_ssize_t others = info->incoming_refs - accounted; + if (PyUnicodeWriter_Format(writer, + "- %zd reference%s to other objects\n", + others, (others == 1) ? "" : "s") < 0) { + goto error; + } + } + + incoming_ref_report_clear(&report); + return PyUnicodeWriter_Finish(writer); + +error: + if (!PyErr_Occurred()) { + PyErr_SetString(PyExc_RuntimeError, "failed to build region close error message"); + } + incoming_ref_report_clear(&report); + PyUnicodeWriter_Discard(writer); + return NULL; +} + +static int +mermaid_write_node(PyUnicodeWriter *writer, PyObject *obj) +{ + if (Region_Check(obj)) { + bool open = ((TracingRegionObject *)obj)->open; + const char *status = open ? "open" : "closed"; + return PyUnicodeWriter_Format(writer, + "n%p[\\Region
%s
rc=%zd
%p/]", + obj, status, Py_REFCNT(obj), obj); + } + if (Cown_Check(obj)) { + return PyUnicodeWriter_Format(writer, + "n%p([\"Cown
rc=%zd
%p\"])", + obj, Py_REFCNT(obj), obj); + } + return PyUnicodeWriter_Format(writer, + "n%p[\"[%s]
rc=%zd
%p\"]", + obj, Py_TYPE(obj)->tp_name, Py_REFCNT(obj), obj); +} + +static int +mermaid_write_class( + PyUnicodeWriter *writer, + PyObject *obj, + _Py_hashtable_t *problem_objs, + _Py_hashtable_t *reported_objs) +{ + if (_Py_IsImmutable(obj)) { + return PyUnicodeWriter_Format(writer, " class n%p immutable\n", obj); + } + if (_Py_hashtable_get_entry(reported_objs, obj) != NULL) { + return PyUnicodeWriter_Format(writer, " class n%p error\n", obj); + } + if (_Py_hashtable_get_entry(problem_objs, obj) != NULL) { + return PyUnicodeWriter_Format(writer, " class n%p problem\n", obj); + } + return 0; +} + +static int +mermaid_write_escaped_label(PyUnicodeWriter *writer, const char *label) +{ + for (const char *p = label; *p != '\0'; p++) { + switch (*p) { + case '|': + if (PyUnicodeWriter_WriteChar(writer, '/') < 0) { + return -1; + } + break; + case '\n': + case '\r': + if (PyUnicodeWriter_WriteChar(writer, ' ') < 0) { + return -1; + } + break; + default: + if (PyUnicodeWriter_WriteChar(writer, (Py_UCS4)(unsigned char)*p) < 0) { + return -1; + } + break; + } + } + return 0; +} + +static int +mermaid_write_escaped_unicode_label(PyUnicodeWriter *writer, PyObject *label) +{ + Py_ssize_t size; + const char *utf8 = PyUnicode_AsUTF8AndSize(label, &size); + if (utf8 == NULL) { + return -1; + } + + Py_ssize_t start = 0; + for (Py_ssize_t i = 0; i < size; i++) { + switch (utf8[i]) { + case '|': + if (i > start && PyUnicodeWriter_WriteUTF8(writer, utf8 + start, i - start) < 0) { + return -1; + } + if (PyUnicodeWriter_WriteChar(writer, '/') < 0) { + return -1; + } + start = i + 1; + break; + case '\n': + case '\r': + if (i > start && PyUnicodeWriter_WriteUTF8(writer, utf8 + start, i - start) < 0) { + return -1; + } + if (PyUnicodeWriter_WriteChar(writer, ' ') < 0) { + return -1; + } + start = i + 1; + break; + default: + break; + } + } + if (size > start && PyUnicodeWriter_WriteUTF8(writer, utf8 + start, size - start) < 0) { + return -1; + } + return 0; +} + +static int +mermaid_enqueue_if_needed(mermaid_dump_state_t *state, PyObject *obj) +{ + if (_Py_IsImmutable(obj) || Cown_Check(obj)) { + return 0; + } + if (Region_Check(obj) && state->src != NULL) { + return 0; + } + if (_Py_hashtable_get_entry(state->visited, obj) != NULL) { + return 0; + } + if (_Py_hashtable_set(state->visited, obj, obj) < 0) { + return -1; + } + return PyList_Append(state->pending, obj); +} + +static int +mermaid_visit_labeled( + PyObject *obj, + mermaid_dump_state_t *state, + const char *ascii_label, + PyObject *unicode_label) +{ + if (_Py_IsImmutable(obj) && ERROR_MERMAID_HIDE_IMMUTABLE && !Cown_Check(obj)) { + return 0; + } + + if (state->src != NULL) { + if (PyUnicodeWriter_WriteUTF8(state->writer, " ", -1) < 0) { + return -1; + } + if (mermaid_write_node(state->writer, state->src) < 0) { + return -1; + } + if (ascii_label != NULL || unicode_label != NULL) { + if (PyUnicodeWriter_WriteUTF8(state->writer, " -->|", -1) < 0) { + return -1; + } + if (ascii_label != NULL) { + if (mermaid_write_escaped_label(state->writer, ascii_label) < 0) { + return -1; + } + } + if (unicode_label != NULL && mermaid_write_escaped_unicode_label(state->writer, unicode_label) < 0) { + return -1; + } + if (PyUnicodeWriter_WriteUTF8(state->writer, "| ", -1) < 0) { + return -1; + } + } + else if (PyUnicodeWriter_WriteUTF8(state->writer, " --> ", -1) < 0) { + return -1; + } + } else if (PyUnicodeWriter_WriteUTF8(state->writer, " ", -1) < 0) { + return -1; + } + + if (mermaid_write_node(state->writer, obj) < 0) { + return -1; + } + if (PyUnicodeWriter_WriteUTF8(state->writer, "\n", -1) < 0) { + return -1; + } + if (mermaid_write_class(state->writer, obj, state->problem_objs, state->reported_objs) < 0) { + return -1; + } + + return mermaid_enqueue_if_needed(state, obj); +} + +static int +mermaid_visit(PyObject *obj, mermaid_dump_state_t *state) +{ + return mermaid_visit_labeled(obj, state, NULL, NULL); +} + +static int +mermaid_visit_dict(PyObject *obj, mermaid_dump_state_t *state) +{ + Py_ssize_t pos = 0; + PyObject *key; + PyObject *value; + + while (PyDict_Next(obj, &pos, &key, &value)) { + if (!_PyImmutability_CanViewAsImmutable(key) + && !Cown_Check(key) + && !Region_Check(key) + ) { + if (mermaid_visit_labeled(key, state, "", NULL) < 0) { + return -1; + } + } + + PyObject *label = PyUnicode_Check(key) ? key : NULL; + if (mermaid_visit_labeled(value, state, NULL, label) < 0) { + return -1; + } + } + return 0; +} + +static int +mermaid_visit_sequence(PyObject *obj, mermaid_dump_state_t *state) +{ + Py_ssize_t size = PyList_CheckExact(obj) ? PyList_GET_SIZE(obj) : PyTuple_GET_SIZE(obj); + for (Py_ssize_t i = 0; i < size; i++) { + char label[32]; + PyOS_snprintf(label, sizeof(label), "#91;%zd#93;", i); + PyObject *item = PyList_CheckExact(obj) ? PyList_GET_ITEM(obj, i) : PyTuple_GET_ITEM(obj, i); + if (mermaid_visit_labeled(item, state, label, NULL) < 0) { + return -1; + } + } + return 0; +} + +static int +mermaid_traverse(PyObject *obj, mermaid_dump_state_t *state) +{ + if (PyDict_CheckExact(obj)) { + return mermaid_visit_dict(obj, state); + } + if (PyList_CheckExact(obj) || PyTuple_CheckExact(obj)) { + return mermaid_visit_sequence(obj, state); + } + + // The trace already reports the types without tp_reachable; the graph dump + // walks the same objects and would only repeat it. + traverseproc proc = get_reachable_proc(Py_TYPE(obj), NULL); + return proc(obj, (visitproc)mermaid_visit, (void *)state); +} + +static void +mermaid_dump_state_destroy(mermaid_dump_state_t *state) +{ + if (state->writer != NULL) { + PyUnicodeWriter_Discard(state->writer); + state->writer = NULL; + } + Py_CLEAR(state->pending); + if (state->visited != NULL) { + _Py_hashtable_destroy(state->visited); + state->visited = NULL; + } +} + +static int +mermaid_dump_state_init( + mermaid_dump_state_t *state, + _Py_hashtable_t *problem_objs, + _Py_hashtable_t *reported_objs) +{ + state->writer = NULL; + state->visited = NULL; + state->pending = NULL; + state->src = NULL; + state->problem_objs = problem_objs; + state->reported_objs = reported_objs; + + state->writer = PyUnicodeWriter_Create(0); + if (state->writer == NULL) { + goto error; + } + state->visited = _Py_hashtable_new( + _Py_hashtable_hash_ptr, + _Py_hashtable_compare_direct); + if (state->visited == NULL) { + goto error; + } + state->pending = PyList_New(0); + if (state->pending == NULL) { + goto error; + } + return 0; + +error: + mermaid_dump_state_destroy(state); + return -1; +} + +static int +dump_mermaid_diagram( + PyObject *root, + _Py_hashtable_t *problem_objs, + _Py_hashtable_t *reported_objs) +{ + int res = -1; + mermaid_dump_state_t state; + PyObject *diagram = NULL; + // Owns the item currently being traversed, released at `finally`. + PyObject *item = NULL; + + // Writing a file into the working directory is too surprising to do by + // default, so the graph is only dumped when it has been asked for. The + // value of the variable is the path to write to. + const char *path = Py_GETENV(REGION_GRAPH_ENV_VAR); + if (path == NULL || *path == '\0') { + return 0; + } + + if (mermaid_dump_state_init(&state, problem_objs, reported_objs) < 0) { + return -1; + } + + if (PyUnicodeWriter_WriteUTF8(state.writer, "flowchart TD\n", -1) < 0) { + goto finally; + } + if (mermaid_visit(root, &state) < 0) { + goto finally; + } + + while (PyList_GET_SIZE(state.pending) > 0) { + Py_XSETREF(item, list_pop(state.pending)); + if (item == NULL) { + goto finally; + } + state.src = item; + SUCCEEDS(mermaid_traverse(item, &state)); + } + Py_CLEAR(item); + + diagram = PyUnicodeWriter_Finish(state.writer); + state.writer = NULL; + if (diagram == NULL) { + goto finally; + } + + const char *body = PyUnicode_AsUTF8(diagram); + if (body == NULL) { + goto finally; + } + + FILE *f = fopen(path, "w"); + if (f == NULL) { + PyErr_SetFromErrnoWithFilename(PyExc_OSError, path); + goto finally; + } + if (fputs( + "
\n" + "\n" + "```mermaid\n" + "%%{init: {'theme': 'neutral', 'themeVariables': { 'fontSize': '16px' }}}%%\n" + "\n", + f) < 0 + || fputs(body, f) < 0 + || fputs( + "\n" + "classDef immutable fill:#94f7ff\n" + "classDef problem fill:#ffe8d6,stroke:#f08c00,stroke-width:2px\n" + "classDef error fill:#ffe8d6,stroke:red,stroke-width:4px\n" + "```\n" + "
\n", + f) < 0) + { + PyErr_SetFromErrnoWithFilename(PyExc_OSError, path); + fclose(f); + goto finally; + } + // Buffered writes can still fail here, so this result matters too. + if (fclose(f) != 0) { + PyErr_SetFromErrnoWithFilename(PyExc_OSError, path); + goto finally; + } + + res = 0; + +finally: + Py_XDECREF(item); + mermaid_dump_state_destroy(&state); + Py_XDECREF(diagram); + return res; +error: + goto finally; +} + +static int _move_obj(PyObject* obj, region_trace_state_t* state) { + // Check the movability of the object: + movable_status status = get_movable_status(obj); + switch (status) { + case Py_MOVABLE_YES: + break; + case Py_MOVABLE_NO: + dbg(" - %p is not movable", obj); + throw_region_error( + "Instances of type '%s' are not movable", Py_TYPE(obj)->tp_name, + state->src, obj); + return TRACE_RES_ERR; + case Py_MOVABLE_FREEZE: + // Freeze the object, this can invalidate our `external_rc`, + // we restart after this trace + dbg(" - freezing %p", obj); + if (_PyImmutability_Freeze(obj)) { + return TRACE_RES_ERR; + } + + region_trace_state_set_restart(state); + return 0; + case Py_MOVABLE_COWN: + return 0; + default: + Py_UNREACHABLE(); + } + + // References to the bridge object are allowed and counted by + // `state->bridge_rc` instead. `_trace_visit()` intercepts them, so the + // bridge must never end up in `visited` or in the LRC below. + assert(obj != state->bridge); + + // Update the LRC, -1 for the reference we just followed + Py_ssize_t lrc_change = Py_REFCNT(obj) - 1; + dbg(" - moving %p; LRC += %zd", obj, lrc_change); + state->external_rc += lrc_change; + + // Mark the object as visited, this stores the lrc_change for better error reporting + if (_Py_hashtable_set(state->visited, obj, (void*)lrc_change) == -1) { + return -1; + } + + // This moves the object into the region list, if provided. + if (state->gc_list && PyObject_IS_GC(obj) && PyObject_GC_IsTracked(obj)) { + // This flag may be set if the region is constructed as part of + // a finalizer. If the flag remains set, for an object removed + // from its GC list bad things can happen. + gc_clear_collecting(_Py_AS_GC(obj)); + // Clearing the space flag makes it easy to merge this list back + // into the local GC lists + gc_set_old_space(_Py_AS_GC(obj), 0); + gc_list_move(_Py_AS_GC(obj), state->gc_list); + } + + // Bridge objects of sub-regions are moved, but shouldn't be traversed. + if (!Region_Check(obj)) { + if (PyList_Append(state->pending, obj)) { + return -1; + } + } + + return 0; +} + +static int _trace_visit(PyObject* obj, region_trace_state_t* state) { + // References to immutable objects are allowed + if (_PyImmutability_CanViewAsImmutable(obj)) { + assert(_Py_IsImmutable(obj)); + return 0; + } + + // References to the bridge are tracked separately + if (obj == state->bridge) { + // This branch also accounts for references from the bridge object to itself. + dbg(" - Internal reference to bridge from %p; bridge_rc += 1", state->src); + state->bridge_rc += 1; + return 0; + } + + // Check if the object is already part of the region + _Py_hashtable_entry_t *entry = _Py_hashtable_get_entry(state->visited, (void*)obj); + if (entry != NULL) { + entry->value = (void*)(((Py_ssize_t)entry->value) - 1); + dbg(" - Internal reference to %p; LRC -= 1", obj); + state->external_rc -= 1; + return 0; + } + + // References external regions turns them into sub-regions. These + // need to be traversed and closed separately + if (Region_Check(obj)) { + if (_PyTracingRegion_IsClosed(obj)) { + // If the child region is closed we can move it directly + return _move_obj(obj, state); + } else { + // The child region is open, we need to traverse it first and then + // retry closing this. + if (PyList_Append(state->tree_trace_state->pending, obj) < 0) { + return -1; + } + region_trace_state_set_restart(state); + } + return 0; + } + + return _move_obj(obj, state); +} + + +static int _try_close_region(PyObject *region_obj, tree_trace_state_t *tree_trace_state) { + assert(Region_Check(region_obj)); + TracingRegionObject* region = (TracingRegionObject*)region_obj; + + // Finalized regions can't be closed since they're deletion would not call the + // finalizer and therefore leak the owned nodes. + if (_PyGC_FINALIZED(region_obj)) { + PyErr_Format( + PyExc_RuntimeError, + "the region %p has been finalized and cannot be closed again", + (void *)region_obj); + return TRACE_RES_ERR; + } + + // Init trace state. + region_trace_state_t state; + if (region_trace_state_init(&state, _PyObject_CAST(region), ®ion->gc_list, tree_trace_state)) { + return TRACE_RES_ERR; + } + int region_trace_res = TRACE_RES_DONE; + // Owns the item currently being traversed, released at `finally`. + PyObject *item = NULL; + + SUCCEEDS(PyList_Append(state.pending, _PyObject_CAST(region))); + + while (PyList_GET_SIZE(state.pending) > 0) { + // Find the next pending item: + Py_XSETREF(item, list_pop(state.pending)); + if (item == NULL) { + goto error; + } + + // Traverse item + state.src = item; + dbg(" - traversing %p", item); + traverseproc proc = get_reachable_proc(Py_TYPE(item), tree_trace_state->missing_reachable); + SUCCEEDS(proc(item, (visitproc)_trace_visit, (void*)&state)); + + // TODO(regions): Handle weakrefs + assert(!PyWeakref_Check(item)); + } + Py_CLEAR(item); + + if (state.restart) { + gc_list_dissolve(®ion->gc_list); + region_trace_res = TRACE_RES_RESTART; + goto finally; + } + + if (state.external_rc == 0) { + _region_close(region, state.bridge_rc, state.visited); + } else { + gc_list_dissolve(®ion->gc_list); + + dbg("- Failed to close region %p, there are %zd incoming references", region, state.external_rc); + close_error_info_t error_info = {0}; + if (close_error_info_init(&error_info, &state) < 0) { + goto error; + } + if (_Py_hashtable_len(state.visited) < ERROR_MERMAID_REPORT_LIMIT) { + // Borrowed error tables; dump_mermaid_diagram() does not take ownership. + if (dump_mermaid_diagram( + region_obj, + error_info.problem_objs, + error_info.reported_objs) < 0) { + // The graph is a diagnostic aid. Report why it is missing, but + // don't let that replace the region error being built here. + PyErr_FormatUnraisable( + "Exception ignored while writing the region graph"); + } + } + + PyObject *msg = build_close_error_message(&error_info); + close_error_info_destroy(&error_info); + if (msg == NULL) { + goto error; + } + PyErr_SetObject(PyExc_RuntimeError, msg); + Py_DECREF(msg); + goto error; + } + + goto finally; +error: + region_trace_res = TRACE_RES_ERR; +finally: + Py_CLEAR(item); + region_trace_state_destroy(&state); + + return region_trace_res; +} + +static int try_close_region_tree(PyObject *root) { + dbg("Starting region tree trace from %p", root); + + tree_trace_state_t state; + if (tree_trace_state_init(&state)) { + return -1; + } + + int tree_trace_res = TRACE_RES_DONE; + + SUCCEEDS(PyList_Append(state.pending, root)); + + while (PyList_GET_SIZE(state.pending) > 0) { + // Look at the region on top of the stack without removing it. A region + // stays queued until it is closed, so the sub-regions that its trace + // discovers end up above it and are closed first. Draining the stack + // therefore means every region in the tree is closed, which is what lets + // this function report success. + Py_ssize_t top = PyList_GET_SIZE(state.pending) - 1; + PyObject *region = PyList_GET_ITEM(state.pending, top); + assert(Region_Check(region)); + + // A closed region has nothing left to do. Regions can be queued more + // than once, this handles all safe cases. + if (_PyTracingRegion_IsClosed(region)) { + SUCCEEDS(PyList_SetSlice(state.pending, top, top + 1, NULL)); + continue; + } + + // Account for this attempt before running it. Counting afterwards would + // report a region that was closed by its last attempt as a failure, and + // would grant `PER_REGION_TRACE_LIMIT + 1` attempts. + _Py_hashtable_entry_t *entry = _Py_hashtable_get_entry(state.tracing_counts, (void*)region); + if (entry == NULL) { + SUCCEEDS(_Py_hashtable_set(state.tracing_counts, (void*)region, (void*)1)); + } else if ((Py_uintptr_t)entry->value < PER_REGION_TRACE_LIMIT) { + entry->value = (void*)(((Py_uintptr_t)entry->value) + 1); + } else { + // FIXME(regions): It would be nicer to spend the last attempt on a + // trace that reports the objects keeping the region open, like the + // `external_rc != 0` path in `_try_close_region()` does, instead of + // this bare message. The catch is that such a trace may close the + // region after all, which is why it can't simply be run here. + PyErr_Format( + PyExc_RuntimeError, + "the region %p could not be closed after %d tracing attempts", + (void *)region, + PER_REGION_TRACE_LIMIT); + goto error; + } + + dbg("- tracing region %p", region); + int res = _try_close_region(region, &state); + if (res == TRACE_RES_ERR) { + goto error; + } + // A restarted trace leaves the region open on purpose. It keeps its slot + // on the stack and is retried once the sub-regions that its trace pushed + // on top of it have been closed. + assert(res == TRACE_RES_RESTART || _PyTracingRegion_IsClosed(region)); + } + + goto finally; +error: + tree_trace_res = TRACE_RES_ERR; +finally: + report_missing_reachable(&state); + tree_trace_state_destroy(&state); + + return tree_trace_res; +} + +// ################################################################### +// Region Object +// ################################################################### + +static PyObject * +TracingRegion_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) { + TracingRegionObject *self = (TracingRegionObject *)type->tp_alloc(type, 0); + if (self == NULL) { + return NULL; + } + + // The region is set up here rather than in `tp_init()`, so that a region + // can never be observed in an uninitialized state. + gc_list_init(&self->gc_list); + // We make the region open by default, this ensures that the first close + // will handle the region type correctly. Alternatively, we could make them + // closed in the beginning, but then handle the cases specifically. + self->open = true; + + return (PyObject *)self; +} + +static int +TracingRegion_init(TracingRegionObject *self, PyObject *args, PyObject *kwargs) { + // `tp_new()` already set the region up. Re-running the initialization here + // would reset the GC list holding the contents of a closed region and drop + // the reference count that `_region_close()` subtracted from the bridge + // object, so this only validates the arguments. + if (!_PyArg_NoPositional("TracingRegion", args) + || !_PyArg_NoKeywords("TracingRegion", kwargs)) + { + return -1; + } + return 0; +} + +/* Disposes of everything a closed region owns. + * + * Closing a region establishes that no object inside it has incoming references + * from the outside; only the bridge object may have those. So once the bridge + * object dies, every member of the region is garbage too, however the references + * between them happen to be arranged. + * + * That lets the region clean up after itself instead of handing the objects back + * to the GC. + * + * This can resurrect the bridge object, so it has to run as a finalizer. + */ +static void _region_delete_contents(TracingRegionObject *self) { + assert(!self->open); + + dbg("Deleting the contents of region %p", self); + + PyGC_Head members; + PyGC_Head survivors; + gc_list_init(&members); + gc_list_init(&survivors); + + // Steal the members and open the region first. A finalizer reaching the + // bridge calls `_open_region()`, which would otherwise dissolve the very + // list being disposed of here. + gc_list_merge(&self->gc_list, &members); + assert(gc_list_is_empty(&self->gc_list)); + // Has to happen before anything is released, the members still hold these. + _restore_internal_bridge_refs(self); + self->open = true; + + // The disposal needs a clean error state; a dealloc can happen mid-raise. + PyObject *exc = PyErr_GetRaisedException(); + + // Cleaning the dict should deallocate most things. + Py_CLEAR(self->dict); + + // Deallocate remaining cyclic garbage + _PyGC_FinalizeGarbage(&members); + _PyGC_DeleteGarbage(&members, &survivors); + PyErr_SetRaisedException(exc); + + // Anything a finalizer kept alive is not owned by the region any more. + if (!gc_list_is_empty(&survivors)) { + gc_list_dissolve(&survivors); + } + // Nothing may still point at these stack allocated list heads. + assert(gc_list_is_empty(&members)); + assert(gc_list_is_empty(&survivors)); +} + +static int +TracingRegion_traverse(TracingRegionObject *self, visitproc visit, void *arg) { + Py_VISIT(self->dict); + return 0; +} + +static int +TracingRegion_clear(TracingRegionObject *self) { + _open_region(self); + Py_CLEAR(self->dict); + return 0; +} + +static void +TracingRegion_finalize(PyObject *op) { + TracingRegionObject *self = (TracingRegionObject *)op; + + if (self->open) { + assert(gc_list_is_empty(&self->gc_list)); + // An open region does not own its members. They live in the GC + // generations and the usual reference counting disposes of them. + Py_CLEAR(self->dict); + } else { + // Objects in a closed region have no incoming references besides the + // one from the bridge. We can therefore delete all objects directly + // instead of returning them to the GC. + _region_delete_contents(self); + } +} + +static void +TracingRegion_dealloc(TracingRegionObject *self) { + PyObject *op = (PyObject *)self; + + // `PyObject_CallFinalizerFromDealloc()` requires a GC type to be tracked + // while the finalizer runs, but the bridge object of a closed region may + // get untracked by an owning cown. + if (!_PyObject_GC_IS_TRACKED(op)) { + _PyObject_GC_TRACK(op); + } + if (PyObject_CallFinalizerFromDealloc(op) < 0) { + // The bridge object was resurrected by the references from inside the + // region. It is deallocated again once those are gone. + return; + } + + // Make sure any objects added after/during finalization are freed + Py_CLEAR(self->dict); + + PyObject_GC_UnTrack(self); + Py_TYPE(self)->tp_free(op); +} + +static PyObject * +TracingRegion_repr(PyObject *op) { + TracingRegionObject *self = (TracingRegionObject*)op; + + // Deliberately reads `open` instead of going through the attribute access + // below, so that reporting on a region does not open it. Deliberately + // address free as well, so that error messages are reproducible. + return PyUnicode_FromFormat( + "", self->open ? "open" : "closed"); +} + +static PyObject * +TracingRegion_getattro(PyObject *op, PyObject *name) { + TracingRegionObject *self = (TracingRegionObject*)op; + _open_region(self); + + return _PyObject_GenericGetAttrWithDict(op, name, self->dict, 0); +} + +static int +TracingRegion_setattro(PyObject *op, PyObject *name, PyObject *value) { + TracingRegionObject *self = (TracingRegionObject*)op; + _open_region(self); + + // Allocate lazily because the generic helper only stores into a provided dict. + if (self->dict == NULL) { + self->dict = PyDict_New(); + if (self->dict == NULL) { + return -1; + } + } + + return _PyObject_GenericSetAttrWithDict(op, name, value, self->dict); +} + +static PyObject * +TracingRegion_get_dict(PyObject *op, void *Py_UNUSED(context)) { + TracingRegionObject *self = (TracingRegionObject*)op; + _open_region(self); + + if (self->dict == NULL) { + self->dict = PyDict_New(); + if (self->dict == NULL) { + return NULL; + } + } + return Py_NewRef(self->dict); +} + +static int +TracingRegion_set_dict(PyObject *op, PyObject *value, void *Py_UNUSED(context)) { + TracingRegionObject *self = (TracingRegionObject*)op; + _open_region(self); + + if (value == NULL) { + PyErr_SetString(PyExc_TypeError, "cannot delete __dict__"); + return -1; + } + if (!PyDict_Check(value)) { + PyErr_Format(PyExc_TypeError, + "__dict__ must be set to a dictionary, not a '%.200s'", + Py_TYPE(value)->tp_name); + return -1; + } + Py_XSETREF(self->dict, Py_NewRef(value)); + return 0; +} + + +/* This method traces the region and closes it, if there are no references + * pointing into the region. References to the bridge are allowed. + * + * This function requires the GIL to be held. + * + * Returns -1 if an exception was raised. 0 if the region could be closed. + */ +int _PyTracingRegion_Close(PyObject* op) { + TracingRegionObject *self = (TracingRegionObject*)op; + if (!self->open) { + return 0; + } + assert(gc_list_is_empty(&self->gc_list)); + + return try_close_region_tree(op); +} + +int _PyTracingRegion_IsClosed(PyObject* region) { + TracingRegionObject *self = (TracingRegionObject*)region; + return !self->open; +} + +static PyMethodDef TracingRegion_methods[] = { + {NULL, NULL} /* sentinel */ +}; + +static PyGetSetDef TracingRegion_getset[] = { + {"__dict__", TracingRegion_get_dict, TracingRegion_set_dict}, + {NULL} +}; + +PyTypeObject _PyTracingRegion_Type = { + PyVarObject_HEAD_INIT(NULL, 0) + .tp_name = "TracingRegion", + .tp_basicsize = sizeof(TracingRegionObject), + .tp_dealloc = (destructor)TracingRegion_dealloc, + .tp_repr = TracingRegion_repr, + .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_IMMUTABLETYPE, + .tp_traverse = (traverseproc)TracingRegion_traverse, + .tp_clear = (inquiry)TracingRegion_clear, + .tp_getset = TracingRegion_getset, + .tp_methods = TracingRegion_methods, + .tp_getattro = TracingRegion_getattro, + .tp_setattro = TracingRegion_setattro, + .tp_init = (initproc)TracingRegion_init, + .tp_new = TracingRegion_new, + .tp_finalize = TracingRegion_finalize, + .tp_reachable = _PyObject_ReachableVisitTypeAndTraverse, +}; + +// TODO: Weak-references part of the trace are not handled diff --git a/PCbuild/_freeze_module.vcxproj b/PCbuild/_freeze_module.vcxproj index 3702e4e99987183..c19b7efdd181537 100644 --- a/PCbuild/_freeze_module.vcxproj +++ b/PCbuild/_freeze_module.vcxproj @@ -134,6 +134,7 @@ + @@ -161,6 +162,7 @@ + diff --git a/PCbuild/_freeze_module.vcxproj.filters b/PCbuild/_freeze_module.vcxproj.filters index 0b968eba5b977bf..0a33235d9dc855c 100644 --- a/PCbuild/_freeze_module.vcxproj.filters +++ b/PCbuild/_freeze_module.vcxproj.filters @@ -106,6 +106,9 @@ Source Files + + Source Files + Source Files @@ -478,6 +481,9 @@ Source Files + + Source Files + Source Files diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index d6ce53bbea28245..32d5877122e4948 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -238,6 +238,7 @@ + @@ -532,6 +533,7 @@ + @@ -559,6 +561,7 @@ + diff --git a/PCbuild/pythoncore.vcxproj.filters b/PCbuild/pythoncore.vcxproj.filters index d5351a82741a0fe..0106e8290c20fa5 100644 --- a/PCbuild/pythoncore.vcxproj.filters +++ b/PCbuild/pythoncore.vcxproj.filters @@ -618,6 +618,9 @@ Include\internal + + Include\internal + Include\internal @@ -697,6 +700,8 @@ Include\internal + Include\internal + Include\internal @@ -1207,6 +1212,9 @@ Objects + + Objects + Objects @@ -1273,6 +1281,9 @@ Objects + + Objects + Objects diff --git a/Python/gc.c b/Python/gc.c index 91f50486cda01ce..67d2a6fcb01262c 100644 --- a/Python/gc.c +++ b/Python/gc.c @@ -968,7 +968,7 @@ handle_weakref_callbacks(PyGC_Head *unreachable, PyGC_Head *old) * Since the callback is never needed and may be unsafe in this * case, wr is simply left in the unreachable set. Note that * clear_weakrefs() will ensure its callback will not trigger - * inside delete_garbage(). + * inside _PyGC_DeleteGarbage(). * * OTOH, if wr isn't part of CT, we should invoke the callback: the * weakref outlived the trash. Note that since wr isn't CT in this @@ -1136,9 +1136,10 @@ handle_legacy_finalizers(PyThreadState *tstate, * Note that this may remove some (or even all) of the objects from the * list, due to refcounts falling to 0. */ -static void -finalize_garbage(PyThreadState *tstate, PyGC_Head *collectable) +void +_PyGC_FinalizeGarbage(PyGC_Head *collectable) { + PyThreadState *tstate = _PyThreadState_GET(); destructor finalize; PyGC_Head seen; @@ -1173,10 +1174,12 @@ finalize_garbage(PyThreadState *tstate, PyGC_Head *collectable) * tricky business as the lists can be changing and we don't know which * objects may be freed. It is possible I screwed something up here. */ -static void -delete_garbage(PyThreadState *tstate, GCState *gcstate, - PyGC_Head *collectable, PyGC_Head *old) +void +_PyGC_DeleteGarbage(PyGC_Head *collectable, PyGC_Head *old) { + PyThreadState *tstate = _PyThreadState_GET(); + GCState *gcstate = &tstate->interp->gc; + assert(!_PyErr_Occurred(tstate)); while (!gc_list_is_empty(collectable)) { @@ -1796,7 +1799,7 @@ gc_collect_region(PyThreadState *tstate, validate_list(&unreachable, collecting_set_unreachable_clear); /* Call tp_finalize on objects which have one. */ - finalize_garbage(tstate, &unreachable); + _PyGC_FinalizeGarbage(&unreachable); /* Handle any objects that may have resurrected after the call * to 'finalize_garbage' and continue the collection with the * objects that are still unreachable */ @@ -1814,7 +1817,7 @@ gc_collect_region(PyThreadState *tstate, * in finalizers to be freed. */ stats->collected += gc_list_size(&final_unreachable); - delete_garbage(tstate, gcstate, &final_unreachable, to); + _PyGC_DeleteGarbage(&final_unreachable, to); /* Collect statistics on uncollectable objects found and print * debugging information. */ diff --git a/Python/immutability.c b/Python/immutability.c index c4feb45d0511c7b..9ca0901159beada 100644 --- a/Python/immutability.c +++ b/Python/immutability.c @@ -1828,6 +1828,14 @@ int _PyImmutability_CanViewAsImmutable(PyObject *obj) } _Py_hashtable_destroy(state.visited); + + // We can't call the destructor directly as we didn't newref the objects + // on push. Breaking out of the loop above leaves the remaining objects + // on the worklist, so drain it here. This is a slow path if there are + // still objects in the stack, so there is no need to optimize it. + while (PyList_Size(state.worklist) > 0) { + pop(state.worklist); + } Py_DECREF(state.worklist); if (result < 0) { diff --git a/region-error-plan.md b/region-error-plan.md new file mode 100644 index 000000000000000..01b1e1874beed7d --- /dev/null +++ b/region-error-plan.md @@ -0,0 +1,158 @@ +## Plan: restore region close diagnostics + +### Current state + +The tree-region rewrite moved the close algorithm into `try_close_region_tree()` and `_try_close_region()` in `Objects/tracingregionobject.c`. The good news is that the important accounting still exists locally: + +- `region_trace_state_t.visited` still maps each moved object to its local refcount delta. +- `region_trace_state_t.external_rc` still carries the total outstanding incoming references for the region being traced. +- `region_trace_state_t.src` still identifies the source object during traversal, which is enough to rebuild graph edges. +- `_try_close_region()` still has the exact failure point where diagnostics should be produced, after dissolving the tentative GC list and after ignoring restart traces. + +The regression is mostly that `_try_close_region()` now formats only: + +```text +Failed to close region %p, there are %zd incoming references +``` + +and then destroys `state.visited`, so the object-level detail and Mermaid graph are lost. The old implementation had these pieces before the rewrite: + +- `error_ref_filter` / `_filter_visited()` to select the first `ERROR_OBJECT_REPORT_COUNT` objects with positive incoming references. +- `build_close_error_message()` to emit: + - `The region could not be closed due to:` + - `- N incoming reference(s) to 'obj'` + - `- N reference(s) to other objects` +- `mermaid_builder_t`, `mermaid_visit()`, and `dump_mermaid_diagram()` to write `region-graph.md` with red-highlighted leaking objects and cyan immutable objects. + +### Desired behavior + +When closing a region tree fails because a particular open region has lingering references into it, the exception should again identify the problematic objects instead of only reporting a total count. For small graphs, the failed close should also regenerate `region-graph.md` so the reference path can be inspected visually. + +The diagnostics should be scoped to the region that actually failed during `try_close_region_tree()`, not to the whole tree unless a later failure aggregation is explicitly added. That preserves the current close algorithm: child regions are closed first; the parent is retried; whichever region still has external refs reports its own graph. + +### Implementation steps + +1. Reintroduce a diagnostic result type. + + Add a small struct near the trace state types, for example: + + ```c + typedef struct { + _Py_hashtable_t *obj_table; + Py_ssize_t incoming_refs; + } close_error_info_t; + ``` + + Keep it separate from `region_trace_state_t` so the tracing state can remain reusable and the caller owns the filtered error table lifetime. + +2. Re-add the filtering helpers, adjusted for bridge semantics. + + Restore the old `error_ref_filter` idea, but make it explicit that the bridge object has one expected external owning reference. In the old code this was handled by subtracting one from the root region object; in the new code the bridge is `state.bridge`. + + Rules for `_filter_visited()`: + + - Start with the stored ref delta from `state.visited`. + - If `key == state.bridge`, subtract the expected owning reference. + - Keep only entries with `refs > 0`. + - Cap the table at `ERROR_OBJECT_REPORT_COUNT` entries. + - Treat `_Py_hashtable_foreach()` return `1` as intentional early stop, not an error. + + This preserves the old message shape while matching the current close model, where references to the bridge from inside the region are tracked separately as `bridge_rc` and should not be reported as external leaks. + +3. Build diagnostics inside `_try_close_region()` before destroying `state`. + + In the `state.external_rc != 0` failure branch, after `gc_list_dissolve(®ion->gc_list)` and after the `state.restart` check: + + - Allocate the filtered `close_error_info_t.obj_table` from `state.visited`. + - Store `close_error_info_t.incoming_refs = state.external_rc`. + - Build the Python exception with the restored `build_close_error_message()`. + - Fall back to the existing summary string only if message construction fails without a more specific exception. + - Destroy the filtered table on all exits. + + Important: do not build the nice error on restart traces. Restart traces are intentionally incomplete because freezing or open child-region discovery invalidated the current accounting. + +4. Restore `build_close_error_message()`. + + Port the old `incoming_ref_report`, `_report_incoming_ref()`, and `build_close_error_message()` almost directly. The main adjustment is replacing `trace_info_t` with `close_error_info_t` and making the expected-reference subtraction happen during filtering, not during final summarization. + + The summary calculation should therefore be: + + ```c + Py_ssize_t problem_refs = error_info->incoming_refs; + ``` + + not `incoming_refs - 1`, because the bridge's expected reference has already been removed from the filtered object counts and should also be excluded from `external_rc` if needed. If `external_rc` still includes the expected bridge reference for the region currently being closed, subtract it once at diagnostic collection time and document that invariant next to the code. + + Cheap check: the existing `test_release_error` expectations in `Lib/test/test_freeze/test_tracing_region.py` should pass with the old exact message lines. + +5. Re-add Mermaid generation as a read-only diagnostic trace. + + Restore `mermaid_builder_t` and `mermaid_visit()`, but adapt it to `region_trace_state_t`: + + - Add `mermaid_builder_t *mermaid;` to `region_trace_state_t`, initialized to `NULL` in `region_trace_state_reset()`. + - At the start of `_trace_visit()`, call `mermaid_visit(obj, state)` when `state->mermaid != NULL`. + - In `mermaid_visit()`, keep the old node format: pointer, refcount, and type name. + - Preserve the old special node shapes for ownership objects: + - Regions use Mermaid's subroutine shape: `id[[Region 0x...]]`. + - Cowns use Mermaid's stadium shape: `id([Cown 0x...])`. + - Continue hiding immutable nodes behind `ERROR_MERMAID_HIDE_IMMUTABLE`. + - Highlight objects present in the filtered error table with `:::error`. + + For the diagnostic trace, initialize `region_trace_state_t` with `gc_list == NULL` so no objects are moved. Use the same `tree_trace_state_t` shape only if required by `_trace_visit()` for region references; otherwise, split a read-only Mermaid visitor path from closing behavior so dumping the graph cannot enqueue or close subregions. + +6. Decide how Mermaid handles sub-regions. + + The tree rewrite adds a case the old graph did not have: references to region bridge objects can represent nested ownership rather than ordinary objects. + + Recommended first version: + + - Show closed sub-region bridge objects as region-shaped boundary nodes and do not traverse into them, matching `_move_obj()`'s current `if (!Region_Check(obj))` behavior. + - Treat open sub-regions as boundary nodes in the graph and label them as `[TracingRegion open]` or `[TracingRegion closed]` if that can be done without allocating risky strings. + - Do not let Mermaid dumping trigger `_enqueue_region_for_closing()` or `region_trace_state_set_restart()`. + - For now, dump only the graph for the single region that failed. Do not attempt to show the whole region tree yet. + + This keeps the diagnostic graph side-effect-free and aligned with the current failure point. A later enhancement can add dashed edges from parent to child region graphs if whole-tree visualization becomes useful. + +7. Write `region-graph.md` only when the graph is small. + + Reuse the old limit: + + ```c + if (_Py_hashtable_len(state.visited) < ERROR_MERMAID_REPORT_LIMIT) { + dump_mermaid_diagram(region_obj, error_info.obj_table); + } + ``` + + Keep the graph dump strictly best-effort: failure to open `region-graph.md` must not replace the close error. Actual Python exceptions from building the diagram should either be cleared and ignored, or avoided by making the dump path best-effort all the way through. For diagnostics, losing the graph is less important than preserving the close failure message. + +8. Add focused tests. + + Update or add tests in `Lib/test/test_freeze/test_tracing_region.py`: + + - Keep the existing simple leak test for exact message shape. + - Add a capped-output test with more than `ERROR_OBJECT_REPORT_COUNT` leaked objects and an `other objects` summary. + - Add a tree-region case where a child region fails to close and the error names an object inside the child, not just the parent total. + - Add a tree-region case where the child closes successfully but the parent fails due to a reference into the parent. + - For Mermaid, either assert that `region-graph.md` exists and contains `flowchart TD` plus `:::error`, or add a small C-visible/private Python hook if file-system assertions are too brittle. + + Also fix the duplicate Python test method name currently present in `TestTraceRefs`; the second `test_release_error` overrides the first. + +9. Validation commands. + + After implementation, run the narrow test file first: + + ```sh + ./python.exe -m test test_freeze.test_tracing_region + ``` + + Then run a build if C changes were made: + + ```sh + make -j + ``` + +### Decisions for this pass + +- `region-graph.md` should show only the single failing region for now. +- Graph dumping is strictly best-effort; diagnostic file failures should not mask the ownership violation. +- Structured exception attributes like `source` and `target` are a follow-up, not part of this restoration pass.