From bba4f1b328aed518cd9b4db7e5cdfa7e3c4bdab7 Mon Sep 17 00:00:00 2001 From: David Klement Date: Fri, 27 Feb 2026 09:06:49 +0100 Subject: [PATCH 1/5] Cowns: Fix switching to GC --- Objects/cownobject.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Objects/cownobject.c b/Objects/cownobject.c index a2dd1a122a3e644..0c1b3f3822fa08d 100644 --- a/Objects/cownobject.c +++ b/Objects/cownobject.c @@ -659,7 +659,11 @@ int _PyCown_SwitchFromIpToGc(_PyCownObject *self, Py_region_t *contained_region) // Cowns holding cowns or immutable objects can be released without any // restrictions if (value_region == _Py_COWN_REGION || value_region == _Py_IMMUTABLE_REGION) { - return cown_release_unchecked(self, ipid); + if (!_Py_atomic_compare_exchange_uint64(&self->owning_ip, &ipid, GC_IPID)) { + *contained_region = NULL_REGION; + return -1; + } + return 0; } assert(value_region != _Py_LOCAL_REGION); From a30b5328806bd90552bd805a62bc1324cf2917fb Mon Sep 17 00:00:00 2001 From: David Klement Date: Fri, 27 Feb 2026 09:26:24 +0100 Subject: [PATCH 2/5] Cowns: Refactor release and switching --- Objects/cownobject.c | 148 +++++++++++++++++++++---------------------- 1 file changed, 71 insertions(+), 77 deletions(-) diff --git a/Objects/cownobject.c b/Objects/cownobject.c index 0c1b3f3822fa08d..05cee779588058e 100644 --- a/Objects/cownobject.c +++ b/Objects/cownobject.c @@ -381,64 +381,76 @@ static int cown_release_unchecked(_PyCownObject* self, _PyCown_ipid_t unlocking_ return 0; } -static int cown_release_region(_PyCownObject* self, _PyCown_ipid_t unlocking_ip) { - assert(_PyRegion_IsBridge(self->value)); - - // If the region is open attempt to close it by cleaning it. - if (_PyRegion_IsOpen(_PyRegion_Get(self->value))) { - int cleaning_res = _PyRegion_Clean(_PyRegion_Get(self->value)); - if (cleaning_res < 0) { - goto error; - } - } - - // A closed region is safe to release - if (!_PyRegion_IsOpen(_PyRegion_Get(self->value))) { - return cown_release_unchecked(self, unlocking_ip); - } - - PyErr_Format( - PyExc_RuntimeError, - "the cown can't be released, since the contained region is still open"); -error: - return -1; -} - -static int cown_release(_PyCownObject *self, _PyCown_ipid_t unlocking_ip) { +/* Check if the cown is not release, and that the owner is as expected. */ +static int cown_check_owner_before_release(_PyCownObject *self, _PyCown_ipid_t unlocking_ip) { _PyCown_ipid_t owning_ip = cown_get_owner(self); - - // Error if the cown is already released if (owning_ip == RELEASED_IPID) { PyErr_Format( PyExc_RuntimeError, - "interpreter %lld attempted to release a released cown", + "interpreter %lld attempted to release/switch a released cown", unlocking_ip ); return -1; } - - // Error if the cown is owned by a different interpreter if (owning_ip != unlocking_ip) { PyErr_Format( PyExc_RuntimeError, - "interpreter %lld attempted to release a cown owned by %lld", + "interpreter %lld attempted to release/switch a cown owned by %lld", unlocking_ip, owning_ip ); return -1; } + return 0; +} - PyObject *value = self->value; +static int cown_is_value_cown_or_immutable(_PyCownObject *self) { + Py_region_t region = _PyRegion_Get(self->value); + return (region == _Py_COWN_REGION || region == _Py_IMMUTABLE_REGION); +} - // Cowns holding cowns or immutable objects can be released without any - // restrictions - Py_region_t value_region = _PyRegion_Get(value); - if (value_region == _Py_COWN_REGION || value_region == _Py_IMMUTABLE_REGION) { +/* Try closing the region by cleaning it. + * Returns: + * (-1) If an error occurred while trying to clean the region. + * (0) If the region is closed after this call. + * (1) If the region is still open after this call. + */ +static int cown_try_closing_region(_PyCownObject *self) { + assert(_PyRegion_IsBridge(self->value)); + Py_region_t region = _PyRegion_Get(self->value); + if (_PyRegion_IsOpen(region)) { + if (_PyRegion_Clean(region) < 0) { + return -1; + } + } + // need to get region again, it might have changed + return _PyRegion_IsOpen(_PyRegion_Get(self->value)); +} + +static int cown_release(_PyCownObject *self, _PyCown_ipid_t unlocking_ip) { + if (cown_check_owner_before_release(self, unlocking_ip) < 0) { + return -1; + } + + if (cown_is_value_cown_or_immutable(self)) { + // Can be released without any restrictions return cown_release_unchecked(self, unlocking_ip); } + assert(_PyRegion_Get(self->value) != _Py_LOCAL_REGION); - assert(value_region != _Py_LOCAL_REGION); - return cown_release_region(self, unlocking_ip); + int cleaning_res = cown_try_closing_region(self); + if (cleaning_res < 0) { + return -1; + } + if (cleaning_res == 1) { + PyErr_Format( + PyExc_RuntimeError, + "the cown can't be released, since the contained region is still open"); + 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) { @@ -640,54 +652,36 @@ int _PyCown_SwitchFromGcToIp(_PyCownObject *self) { return 0; } -/* -* Returns: -* (-1) On error -* (0) On success -* (1) If the cown could not be switched to GC -*/ -int _PyCown_SwitchFromIpToGc(_PyCownObject *self, Py_region_t *contained_region) { - // FIXME(cowns): xFrednet: This could be a lot cleaner and share - // implementations with the normal release function. - _PyCown_ipid_t ipid = _PyCown_ThisInterpreterId(); - BAIL_UNLESS_OWNED_BY(self, ipid, -1); - - PyObject *value = self->value; - Py_region_t value_region = _PyRegion_Get(value); - *contained_region = value_region; - - // Cowns holding cowns or immutable objects can be released without any - // restrictions - if (value_region == _Py_COWN_REGION || value_region == _Py_IMMUTABLE_REGION) { - if (!_Py_atomic_compare_exchange_uint64(&self->owning_ip, &ipid, GC_IPID)) { - *contained_region = NULL_REGION; - return -1; - } - return 0; +int cown_switch_to_gc_unchecked(_PyCownObject *self,_PyCown_ipid_t ipid, Py_region_t *contained_region) { + if (!_Py_atomic_compare_exchange_uint64(&self->owning_ip, &ipid, GC_IPID)) { + return -1; } + *contained_region = _PyRegion_Get(self->value); + return 0; +} - assert(value_region != _Py_LOCAL_REGION); - assert(_PyRegion_IsBridge(self->value)); - - // If the region is open attempt to close it by cleaning it. - if (_PyRegion_IsOpen(_PyRegion_Get(self->value))) { - int cleaning_res = _PyRegion_Clean(_PyRegion_Get(self->value)); - if (cleaning_res < 0) { - return -1; - } +int _PyCown_SwitchFromIpToGc(_PyCownObject *self, Py_region_t *contained_region) { + _PyCown_ipid_t ipid = _PyCown_ThisInterpreterId(); + *contained_region = NULL_REGION; + if (cown_check_owner_before_release(self, ipid) < 0) { + return -1; } - // A closed region is safe to release - if (_PyRegion_IsOpen(_PyRegion_Get(self->value))) { - *contained_region = NULL_REGION; - return 1; + if (cown_is_value_cown_or_immutable(self)) { + // Can be switched without any restrictions + return cown_switch_to_gc_unchecked(self, ipid, contained_region); } + assert(_PyRegion_Get(self->value) != _Py_LOCAL_REGION); - if (!_Py_atomic_compare_exchange_uint64(&self->owning_ip, &ipid, GC_IPID)) { - *contained_region = NULL_REGION; + int clean_res = cown_try_closing_region(self); + if (clean_res < 0) { return -1; } - return 0; + if (clean_res == 1) { + return 1; + } + // Region is closed, safe to switch + return cown_switch_to_gc_unchecked(self, ipid, contained_region); } int _PyCown_ReleaseGC(_PyCownObject *self) { From 0e05f68639a36f001f14e2d9cecea2f62066ff79 Mon Sep 17 00:00:00 2001 From: David Klement Date: Thu, 26 Feb 2026 14:27:26 +0100 Subject: [PATCH 3/5] Cowns: Set value to None if cannot switch to GC --- Objects/cownobject.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Objects/cownobject.c b/Objects/cownobject.c index 05cee779588058e..0b6f66d70ff0c5a 100644 --- a/Objects/cownobject.c +++ b/Objects/cownobject.c @@ -678,8 +678,12 @@ int _PyCown_SwitchFromIpToGc(_PyCownObject *self, Py_region_t *contained_region) return -1; } if (clean_res == 1) { - return 1; - } + // The region is still open, and we won't be able to release the cown. + // After GC, the cown will still be owned by the current interpreter. + // Nobody expects this. + // Replace the cown's value with an exception. + // FIXME(cowns): exceptions cannot yet be frozen, setting None for now + cown_set_value_unchecked(self, Py_None); } // Region is closed, safe to switch return cown_switch_to_gc_unchecked(self, ipid, contained_region); } From d209fe817217f28817e827c3aec77730d4387478 Mon Sep 17 00:00:00 2001 From: xFrednet Date: Fri, 27 Feb 2026 13:09:35 +0100 Subject: [PATCH 4/5] Minor adjustments --- Objects/cownobject.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Objects/cownobject.c b/Objects/cownobject.c index 0b6f66d70ff0c5a..6861b6a5738dd7b 100644 --- a/Objects/cownobject.c +++ b/Objects/cownobject.c @@ -381,7 +381,7 @@ static int cown_release_unchecked(_PyCownObject* self, _PyCown_ipid_t unlocking_ return 0; } -/* Check if the cown is not release, and that the owner is as expected. */ +/* 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) { @@ -422,7 +422,7 @@ static int cown_try_closing_region(_PyCownObject *self) { return -1; } } - // need to get region again, it might have changed + // This needs to get the region again as it might have changed return _PyRegion_IsOpen(_PyRegion_Get(self->value)); } @@ -683,7 +683,8 @@ int _PyCown_SwitchFromIpToGc(_PyCownObject *self, Py_region_t *contained_region) // Nobody expects this. // Replace the cown's value with an exception. // FIXME(cowns): exceptions cannot yet be frozen, setting None for now - cown_set_value_unchecked(self, Py_None); } + cown_set_value_unchecked(self, Py_None); + } // Region is closed, safe to switch return cown_switch_to_gc_unchecked(self, ipid, contained_region); } From 24c67d46cde0641e5f65ba5975cc1134b31a535f Mon Sep 17 00:00:00 2001 From: xFrednet Date: Fri, 27 Feb 2026 13:09:35 +0100 Subject: [PATCH 5/5] Minor adjustments --- Objects/cownobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/cownobject.c b/Objects/cownobject.c index 6861b6a5738dd7b..69cec4bb86ea729 100644 --- a/Objects/cownobject.c +++ b/Objects/cownobject.c @@ -652,7 +652,7 @@ int _PyCown_SwitchFromGcToIp(_PyCownObject *self) { return 0; } -int cown_switch_to_gc_unchecked(_PyCownObject *self,_PyCown_ipid_t ipid, Py_region_t *contained_region) { +int cown_switch_to_gc_unchecked(_PyCownObject *self, _PyCown_ipid_t ipid, Py_region_t *contained_region) { if (!_Py_atomic_compare_exchange_uint64(&self->owning_ip, &ipid, GC_IPID)) { return -1; }