From ee4f5d3726952b02f14f5d0faa111fda1cfb715c Mon Sep 17 00:00:00 2001 From: Emery Conrad Date: Thu, 22 Oct 2026 09:14:22 -0500 Subject: [PATCH] [cpyrt] Raise C++ exceptions from destructors in Python A noexcept(false) destructor that throws aborted the process, because the destructor path did not catch C++ exceptions. __destruct__() now raises the exception as a Python exception. Deallocation reports it through sys.unraisablehook, as CPython does for __del__. Move the exception conversion from CPPMethod into ProxyWrappers, so that both paths use it. Co-developed-with-the-help-of: Claude Code (Opus 5.5, human in the loop) --- src/cpyrt/CPPInstance.cxx | 42 +++++++++++++++++++++++------ src/cpyrt/CPPInstance.h | 2 +- src/cpyrt/CPPMethod.cxx | 54 +------------------------------------ src/cpyrt/ProxyWrappers.cxx | 52 +++++++++++++++++++++++++++++++++++ src/cpyrt/ProxyWrappers.h | 4 +++ test/test_advancedcpp.py | 52 +++++++++++++++++++++++++++++++++++ 6 files changed, 144 insertions(+), 62 deletions(-) diff --git a/src/cpyrt/CPPInstance.cxx b/src/cpyrt/CPPInstance.cxx index ec882a53..779ce9eb 100644 --- a/src/cpyrt/CPPInstance.cxx +++ b/src/cpyrt/CPPInstance.cxx @@ -14,6 +14,7 @@ using namespace cppjit; #include "cppjit_interop.h" #include "cpyrt/DispatchPtr.h" +#include "cpyrt/PyException.h" // Standard #include @@ -213,8 +214,9 @@ void cpyrt::CPPInstance::SetDispatchPtr(void* ptr) { } //---------------------------------------------------------------------------- -void cpyrt::op_dealloc_nofree(CPPInstance* pyobj) { +bool cpyrt::op_dealloc_nofree(CPPInstance* pyobj) { // Destroy the held C++ object, if owned; does not deallocate the proxy. + // Returns false with a Python error set if the destructor throws. interop::TCppScope_t klass = pyobj->ObjectIsA(false /* check_smart */); void*& cppobj = pyobj->GetObjectRaw(); @@ -223,18 +225,36 @@ void cpyrt::op_dealloc_nofree(CPPInstance* pyobj) { MemoryRegulator::UnregisterPyObject(pyobj, (PyObject*)Py_TYPE((PyObject*)pyobj)); + bool ok = true; if (cppobj && (pyobj->fFlags & CPPInstance::kIsOwner)) { - if (pyobj->fFlags & CPPInstance::kIsValue) { - interop::CallDestructor(klass, cppobj); - interop::Deallocate(klass, cppobj); - } else - interop::Destruct(klass, cppobj); + try { + if (pyobj->fFlags & CPPInstance::kIsValue) { + // [expr.delete]: the storage is freed even if the destructor throws + try { + interop::CallDestructor(klass, cppobj); + } catch (...) { + interop::Deallocate(klass, cppobj); + throw; + } + interop::Deallocate(klass, cppobj); + } else + interop::Destruct(klass, cppobj); + } catch (PyException&) { + ok = false; // error already set + } catch (std::exception& e) { + SetPyErrFromCppException(e); + ok = false; + } catch (...) { + PyErr_SetString(PyExc_Exception, "unhandled, unknown C++ exception"); + ok = false; + } } cppobj = nullptr; if (pyobj->IsExtended()) delete (ExtendedData*)pyobj->fObject; pyobj->fFlags = CPPInstance::kNoWrapConv; + return ok; } namespace cppjit::cpyrt { @@ -276,7 +296,8 @@ static PyObject* op_destruct(CPPInstance* self) { // garbage collector (like in PyPy), to allow the user control over when // the C++ destructor is called. This method requires that the C++ object // is owned (no-op otherwise). - op_dealloc_nofree(self); + if (!op_dealloc_nofree(self)) + return nullptr; Py_RETURN_NONE; } @@ -454,7 +475,12 @@ static CPPInstance* op_new(PyTypeObject* subtype, PyObject*, PyObject*) { static void op_dealloc(CPPInstance* pyobj) { // Remove (Python-side) memory held by the object proxy. PyObject_GC_UnTrack((PyObject*)pyobj); - op_dealloc_nofree(pyobj); + // dealloc cannot raise, so report the way CPython reports __del__ errors + PyObject *pytype, *pyvalue, *pytrace; + PyErr_Fetch(&pytype, &pyvalue, &pytrace); + if (!op_dealloc_nofree(pyobj)) + PyErr_WriteUnraisable((PyObject*)Py_TYPE(pyobj)); + PyErr_Restore(pytype, pyvalue, pytrace); PyObject_GC_Del((PyObject*)pyobj); } diff --git a/src/cpyrt/CPPInstance.h b/src/cpyrt/CPPInstance.h index c94ecfa1..8ac5fadf 100644 --- a/src/cpyrt/CPPInstance.h +++ b/src/cpyrt/CPPInstance.h @@ -142,7 +142,7 @@ template inline bool CPPInstance_CheckExact(T* object) { } //- helper for memory regulation (no PyTypeObject equiv. member in p2.2) ----- -void op_dealloc_nofree(CPPInstance*); +bool op_dealloc_nofree(CPPInstance*); } // namespace cppjit::cpyrt diff --git a/src/cpyrt/CPPMethod.cxx b/src/cpyrt/CPPMethod.cxx index 0221f836..6d0705b0 100644 --- a/src/cpyrt/CPPMethod.cxx +++ b/src/cpyrt/CPPMethod.cxx @@ -119,60 +119,8 @@ inline PyObject* cpyrt::CPPMethod::ExecuteFast(void* self, ptrdiff_t offset, ctxt->fFlags |= CallContext::kPyException; result = nullptr; // error already set } catch (std::exception& e) { - // attempt to set the exception to the actual type, to allow catching with - // the Python C++ type - static interop::TCppScope_t exc_type = - interop::GetFullScope("std::exception"); - ctxt->fFlags |= CallContext::kCppException; - - PyObject* pyexc_type = nullptr; - PyObject* pyexc_obj = nullptr; - - // TODO: factor this code with the same in ProxyWrappers (and cache it there - // to be able to look up based on TCppType_t): - interop::TCppScope_t actual = interop::GetActualClass(exc_type, &e); - const std::string& finalname = interop::GetScopedFinalName(actual); - const std::string& parentname = TypeManip::extract_namespace(finalname); - PyObject* parent = CreateScopeProxy(parentname); - if (parent) { - pyexc_type = PyObject_GetAttrString( - parent, - parentname.empty() - ? finalname.c_str() - : finalname.substr(parentname.size() + 2, std::string::npos) - .c_str()); - Py_DECREF(parent); - } - - if (pyexc_type) { - // create a copy of the exception (TODO: factor this code with the same in - // ProxyWrappers) - PyObject* pyclass = cpyrt::GetScopeProxy(actual); - PyObject* source = BindCppObjectNoCast(&e, actual); - PyObject* pyexc_copy = - PyObject_CallFunctionObjArgs(pyclass, source, nullptr); - Py_DECREF(source); - Py_DECREF(pyclass); - if (pyexc_copy) { - pyexc_obj = CPPExcInstance_Type.tp_new((PyTypeObject*)pyexc_type, - nullptr, nullptr); - ((CPPExcInstance*)pyexc_obj)->fCppInstance = (PyObject*)pyexc_copy; - } else - PyErr_Clear(); - } else - PyErr_Clear(); - - if (pyexc_type && pyexc_obj) { - PyErr_SetObject(pyexc_type, pyexc_obj); - Py_DECREF(pyexc_obj); - Py_DECREF(pyexc_type); - } else { - PyErr_Format(PyExc_Exception, "%s (C++ exception)", e.what()); - Py_XDECREF(pyexc_obj); - Py_XDECREF(pyexc_type); - } - + SetPyErrFromCppException(e); result = nullptr; } catch (...) { // don't set the kCppException flag here, as there is basically no useful diff --git a/src/cpyrt/ProxyWrappers.cxx b/src/cpyrt/ProxyWrappers.cxx index 7f804ffb..9f6d8f51 100644 --- a/src/cpyrt/ProxyWrappers.cxx +++ b/src/cpyrt/ProxyWrappers.cxx @@ -1058,3 +1058,55 @@ PyObject* cpyrt::BindCppObjectArray(interop::TCppObject_t address, // TODO: this function exists for symmetry; need to figure out if it's useful return TupleOfInstances_New(address, klass, dims); } + +//---------------------------------------------------------------------------- +void cpyrt::SetPyErrFromCppException(std::exception& e) { + // attempt to set the exception to the actual type, to allow catching with + // the Python C++ type + static interop::TCppScope_t exc_type = + interop::GetFullScope("std::exception"); + + PyObject* pyexc_type = nullptr; + PyObject* pyexc_obj = nullptr; + + // TODO: cache the lookup based on TCppType_t + interop::TCppScope_t actual = interop::GetActualClass(exc_type, &e); + const std::string& finalname = interop::GetScopedFinalName(actual); + const std::string& parentname = TypeManip::extract_namespace(finalname); + PyObject* parent = CreateScopeProxy(parentname); + if (parent) { + pyexc_type = PyObject_GetAttrString( + parent, parentname.empty() + ? finalname.c_str() + : finalname.substr(parentname.size() + 2, std::string::npos) + .c_str()); + Py_DECREF(parent); + } + + if (pyexc_type) { + // create a copy of the exception + PyObject* pyclass = cpyrt::GetScopeProxy(actual); + PyObject* source = BindCppObjectNoCast(&e, actual); + PyObject* pyexc_copy = + PyObject_CallFunctionObjArgs(pyclass, source, nullptr); + Py_DECREF(source); + Py_DECREF(pyclass); + if (pyexc_copy) { + pyexc_obj = CPPExcInstance_Type.tp_new((PyTypeObject*)pyexc_type, nullptr, + nullptr); + ((CPPExcInstance*)pyexc_obj)->fCppInstance = (PyObject*)pyexc_copy; + } else + PyErr_Clear(); + } else + PyErr_Clear(); + + if (pyexc_type && pyexc_obj) { + PyErr_SetObject(pyexc_type, pyexc_obj); + Py_DECREF(pyexc_obj); + Py_DECREF(pyexc_type); + } else { + PyErr_Format(PyExc_Exception, "%s (C++ exception)", e.what()); + Py_XDECREF(pyexc_obj); + Py_XDECREF(pyexc_type); + } +} diff --git a/src/cpyrt/ProxyWrappers.h b/src/cpyrt/ProxyWrappers.h index ab4fec13..d08d9480 100644 --- a/src/cpyrt/ProxyWrappers.h +++ b/src/cpyrt/ProxyWrappers.h @@ -5,6 +5,7 @@ #include "Dimensions.h" // Standard +#include #include namespace cppjit::cpyrt { @@ -32,6 +33,9 @@ PyObject* BindCppObject(interop::TCppObject_t object, PyObject* BindCppObjectArray(interop::TCppObject_t address, interop::TCppScope_t klass, cdims_t dims); +// set the Python error to a proxy of the C++ exception, or a plain Exception +void SetPyErrFromCppException(std::exception& e); + } // namespace cppjit::cpyrt #endif // !CPYRT_PROXYWRAPPERS_H diff --git a/test/test_advancedcpp.py b/test/test_advancedcpp.py index 6bd7aa7b..7953fcc8 100644 --- a/test/test_advancedcpp.py +++ b/test/test_advancedcpp.py @@ -1098,3 +1098,55 @@ def test31_virtual_base_downcast(self): assert isinstance(obj3, ns.Visible3) assert obj3.value == 23 assert obj3.z == 7 + + @mark.xfail( + condition=IS_MAC_ARM, + run=False, + reason="Crashes with exception not being caught on Apple Silicon", + ) + def test33_throwing_destructor(self): + """A noexcept(false) destructor that throws raises in Python""" + + import sys + + import cppjit + + cppjit.cppdef("""\ + #include + namespace throwing_dtor { + int g_count = 0; + struct Throws { + ~Throws() noexcept(false) { + ++g_count; + throw std::runtime_error("from dtor"); + } + }; + Throws make() { return {}; } + }""") + + ns = cppjit.gbl.throwing_dtor + + t = ns.Throws() + with raises(cppjit.gbl.std.runtime_error, match="from dtor"): + t.__destruct__() + assert ns.g_count == 1 + t.__destruct__() # no longer owned: no second destructor call + assert ns.g_count == 1 + + v = ns.make() + with raises(cppjit.gbl.std.runtime_error, match="from dtor"): + v.__destruct__() + assert ns.g_count == 2 + + # dealloc cannot raise: the error goes to sys.unraisablehook + unraisable = [] + old_hook = sys.unraisablehook + sys.unraisablehook = unraisable.append + try: + t = ns.Throws() + del t + finally: + sys.unraisablehook = old_hook + assert ns.g_count == 3 + assert len(unraisable) == 1 + assert "from dtor" in str(unraisable[0].exc_value)