diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index 031484c9d0f..feab4b81bbb 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -559,6 +559,44 @@ crucial that instances are deallocated on the C++ side to avoid memory leaks. py::class_>(m, "MyClass") .def(py::init<>()) +.. _destructors_that_call_python: + +Destructors that call Python +============================ + +If a Python function is invoked from a C++ destructor, an exception may be thrown +of type :class:`error_already_set`. If this error is thrown out of a class destructor, +``std::terminate()`` will be called, terminating the process. Class destructors +must catch all exceptions of type :class:`error_already_set` to discard the Python +exception using :meth:`error_already_set::discard_as_unraisable`. + +Every Python function should be treated as *possibly throwing*. When a Python generator +stops yielding items, Python will throw a ``StopIteration`` exception, which can pass +though C++ destructors if the generator's stack frame holds the last reference to C++ +objects. + +For more information, see :ref:`the documentation on exceptions `. + +.. code-block:: cpp + + class MyClass { + public: + ~MyClass() { + try { + py::print("Even printing is dangerous in a destructor"); + py::exec("raise ValueError('This is an unraisable exception')"); + } catch (py::error_already_set &e) { + // error_context should be information about where/why the occurred, + // e.g. use __func__ to get the name of the current function + e.discard_as_unraisable(__func__); + } + } + }; + +.. note:: + + pybind11 does not support C++ destructors marked ``noexcept(false)``. + .. _implicit_conversions: Implicit conversions diff --git a/docs/advanced/exceptions.rst b/docs/advanced/exceptions.rst index 75ad7f7f4a5..d08ef943509 100644 --- a/docs/advanced/exceptions.rst +++ b/docs/advanced/exceptions.rst @@ -53,9 +53,15 @@ exceptions: | | a Python exception back to Python. | +--------------------------------------+--------------------------------------+ -When a Python function invoked from C++ throws an exception, it is converted -into a C++ exception of type :class:`error_already_set` whose string payload -contains a textual summary. +When a Python function invoked from C++ throws an exception, pybind11 will convert +it into a C++ exception of type :class:`error_already_set` whose string payload +contains a textual summary. If you call the Python C-API directly, and it +returns an error, you should ``throw py::error_already_set();``, which allows +pybind11 to deal with the exception and pass it back to the Python interpreter. +(Another option is to call ``PyErr_Clear`` in the +:ref:`Python C-API ` +to clear the error. The Python error must be thrown or cleared, or Python/pybind11 +will be left in an invalid state.) There is also a special exception :class:`cast_error` that is thrown by :func:`handle::call` when the input arguments cannot be converted to Python @@ -142,3 +148,43 @@ section. Exceptions that you do not plan to handle should simply not be caught, or may be explicitly (re-)thrown to delegate it to the other, previously-declared existing exception translators. + +.. _unraisable_exceptions: + +Handling unraisable exceptions +============================== + +If a Python function invoked from a C++ destructor or any function marked +``noexcept(true)`` (collectively, "noexcept functions") throws an exception, there +is no way to propagate the exception, as such functions may not throw at +run-time. + +Neither Python nor C++ allow exceptions raised in a noexcept function to propagate. In +Python, an exception raised in a class's ``__del__`` method is logged as an +unraisable error. In Python 3.8+, a system hook is triggered and an auditing +event is logged. In C++, ``std::terminate()`` is called to abort immediately. + +Any noexcept function should have a try-catch block that traps +class:`error_already_set` (or any other exception that can occur). Note that pybind11 +wrappers around Python exceptions such as :class:`pybind11::value_error` are *not* +Python exceptions; they are C++ exceptions that pybind11 catches and converts to +Python exceptions. Noexcept functions cannot propagate these exceptions either. +You can convert them to Python exceptions and then discard as unraisable. + +.. code-block:: cpp + + void nonthrowing_func() noexcept(true) { + try { + // ... + } catch (py::error_already_set &eas) { + // Discard the Python error using Python APIs, using the C++ magic + // variable __func__. Python already knows the type and value and of the + // exception object. + eas.discard_as_unraisable(__func__); + } catch (const std::exception &e) { + // Log and discard C++ exceptions. + // (We cannot use discard_as_unraisable, since we have a generic C++ + // exception, not an exception that originated from Python.) + third_party::log(e); + } + } diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 9000668b919..cb9949613ee 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -337,6 +337,22 @@ class error_already_set : public std::runtime_error { /// error variables (but the `.what()` string is still available). void restore() { PyErr_Restore(m_type.release().ptr(), m_value.release().ptr(), m_trace.release().ptr()); } + /// If it is impossible to raise the currently-held error, such as in destructor, we can write + /// it out using Python's unraisable hook (sys.unraisablehook). The error context should be + /// some object whose repr() helps identify the location of the error. Python already knows the + /// type and value of the error, so there is no need to repeat that. For example, __func__ could + /// be helpful. After this call, the current object no longer stores the error variables, + /// and neither does Python. + void discard_as_unraisable(object err_context) { + restore(); + PyErr_WriteUnraisable(err_context.ptr()); + } + void discard_as_unraisable(const char *err_context) { + auto obj = reinterpret_steal(PYBIND11_FROM_STRING(err_context)); + restore(); + PyErr_WriteUnraisable(obj.ptr()); + } + // Does nothing; provided for backwards compatibility. PYBIND11_DEPRECATED("Use of error_already_set.clear() is deprecated") void clear() {} diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 56cd9bc48f7..372d0aebf42 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -65,6 +65,25 @@ struct PythonCallInDestructor { py::dict d; }; + + +struct PythonAlreadySetInDestructor { + PythonAlreadySetInDestructor(const py::str &s) : s(s) {} + ~PythonAlreadySetInDestructor() { + py::dict foo; + try { + // Assign to a py::object to force read access of nonexistent dict entry + py::object o = foo["bar"]; + } + catch (py::error_already_set& ex) { + ex.discard_as_unraisable(s); + } + } + + py::str s; +}; + + TEST_SUBMODULE(exceptions, m) { m.def("throw_std_exception", []() { throw std::runtime_error("This exception was intentionally thrown."); @@ -183,6 +202,11 @@ TEST_SUBMODULE(exceptions, m) { return false; }); + m.def("python_alreadyset_in_destructor", [](py::str s) { + PythonAlreadySetInDestructor alreadyset_in_destructor(s); + return true; + }); + // test_nested_throws m.def("try_catch", [m](py::object exc_type, py::function f, py::args args) { try { f(*args); } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 053e7d4a288..aba7f42b428 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -1,4 +1,6 @@ # -*- coding: utf-8 -*- +import sys + import pytest from pybind11_tests import exceptions as m @@ -48,6 +50,34 @@ def test_python_call_in_catch(): assert d["good"] is True +def test_python_alreadyset_in_destructor(monkeypatch, capsys): + hooked = False + triggered = [False] # mutable, so Python 2.7 closure can modify it + + if hasattr(sys, 'unraisablehook'): # Python 3.8+ + hooked = True + default_hook = sys.unraisablehook + + def hook(unraisable_hook_args): + exc_type, exc_value, exc_tb, err_msg, obj = unraisable_hook_args + if obj == 'PythonAlreadySetInDestructor': + triggered[0] = True + default_hook(unraisable_hook_args) + return + + # Use monkeypatch so pytest can apply and remove the patch as appropriate + monkeypatch.setattr(sys, 'unraisablehook', hook) + + assert m.python_alreadyset_in_destructor('already_set demo') is True + if hooked: + assert triggered[0] is True + + captured = capsys.readouterr() + assert captured.err.startswith( + "Exception ignored in: 'PythonAlreadySetInDestructor'" + ) + + def test_exception_matches(): assert m.exception_matches() assert m.exception_matches_base()