diff --git a/Objects/cownobject.c b/Objects/cownobject.c index a2dd1a122a3e64..69cec4bb86ea72 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) { +/* 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); - - // 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; + } + } + // This needs to get the region again as 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,50 +652,41 @@ 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) { - return cown_release_unchecked(self, ipid); +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) { + // 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); } int _PyCown_ReleaseGC(_PyCownObject *self) {