Skip to content

Commit 8d1e164

Browse files
committed
Add and document py::error_already_set::discard_as_unraisable()
To deal with exceptions that hit destructors or other noexcept functions.
1 parent 227170d commit 8d1e164

4 files changed

Lines changed: 103 additions & 0 deletions

File tree

docs/advanced/classes.rst

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,39 @@ crucial that instances are deallocated on the C++ side to avoid memory leaks.
559559
py::class_<MyClass, std::unique_ptr<MyClass, py::nodelete>>(m, "MyClass")
560560
.def(py::init<>())
561561
562+
.. _destructors_that_call_python:
563+
564+
Destructors that call Python
565+
============================
566+
567+
If a Python function is invoked from a C++ destructor, an exception may be thrown
568+
of type :class:`error_already_set`. If this error is thrown out of a class destructor,
569+
``std::terminate()`` will be called, terminating the process. Class destructors
570+
must catch all exceptions of type :class:`error_already_set` to discard the Python
571+
exception using :meth:`error_already_set::discard_as_unraisable`. (In addition,
572+
you should catch any C++ exceptions that may occur, if any, and deal with them.
573+
Since C++ exceptions are not Python exceptions, you cannot use ``discard_as_unraisable``).
574+
575+
Every Python function, whether called through a pybind11 wrapper or through the
576+
Python C-API, should be treated as *possibly throwing*.
577+
578+
For more information, see :ref:`the documentation on exceptions <unraisable_exceptions>`.
579+
580+
.. code-block:: cpp
581+
582+
class MyClass {
583+
public:
584+
~MyClass() {
585+
try {
586+
py::print("Even printing is dangerous in a destructor");
587+
py::exec("raise ValueError('This is an unraisable exception')");
588+
}
589+
catch (py::error_already_set &e) {
590+
e.discard_as_unraisable(__func__);
591+
};
592+
}
593+
};
594+
562595
.. _implicit_conversions:
563596

564597
Implicit conversions

include/pybind11/pytypes.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,22 @@ class error_already_set : public std::runtime_error {
337337
/// error variables (but the `.what()` string is still available).
338338
void restore() { PyErr_Restore(m_type.release().ptr(), m_value.release().ptr(), m_trace.release().ptr()); }
339339

340+
/// If it is impossible to raise the currently-held error, such as in destructor, we can write
341+
/// it out using Python's unraisable hook (sys.unraisablehook). The error context should be
342+
/// some object whose repr() helps identify the location of the error. Python already knows the
343+
/// type and value of the error, so there is no need to repeat that. For example, __func__ could
344+
/// be helpful. After this call, the current object no longer stores the error variables,
345+
/// and neither does Python.
346+
void discard_as_unraisable(object err_context) {
347+
restore();
348+
PyErr_WriteUnraisable(err_context.ptr());
349+
}
350+
void discard_as_unraisable(const char *err_context) {
351+
auto obj = reinterpret_steal<object>(PYBIND11_FROM_STRING(err_context));
352+
restore();
353+
PyErr_WriteUnraisable(obj.ptr());
354+
}
355+
340356
// Does nothing; provided for backwards compatibility.
341357
PYBIND11_DEPRECATED("Use of error_already_set.clear() is deprecated")
342358
void clear() {}

tests/test_exceptions.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,25 @@ struct PythonCallInDestructor {
6565
py::dict d;
6666
};
6767

68+
69+
70+
struct PythonAlreadySetInDestructor {
71+
PythonAlreadySetInDestructor(const py::str &s) : s(s) {}
72+
~PythonAlreadySetInDestructor() {
73+
py::dict foo;
74+
try {
75+
// Assign to a py::object to force read access of nonexistent dict entry
76+
py::object o = foo["bar"];
77+
}
78+
catch (py::error_already_set& ex) {
79+
ex.discard_as_unraisable("PythonAlreadySetInDestructor");
80+
}
81+
}
82+
83+
py::str s;
84+
};
85+
86+
6887
TEST_SUBMODULE(exceptions, m) {
6988
m.def("throw_std_exception", []() {
7089
throw std::runtime_error("This exception was intentionally thrown.");
@@ -183,6 +202,11 @@ TEST_SUBMODULE(exceptions, m) {
183202
return false;
184203
});
185204

205+
m.def("python_alreadyset_in_destructor", [](py::str s) {
206+
PythonAlreadySetInDestructor alreadyset_in_destructor(s);
207+
return true;
208+
});
209+
186210
// test_nested_throws
187211
m.def("try_catch", [m](py::object exc_type, py::function f, py::args args) {
188212
try { f(*args); }

tests/test_exceptions.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
# -*- coding: utf-8 -*-
2+
import sys
3+
24
import pytest
35

46
from pybind11_tests import exceptions as m
@@ -48,6 +50,34 @@ def test_python_call_in_catch():
4850
assert d["good"] is True
4951

5052

53+
def test_python_alreadyset_in_destructor(monkeypatch, capsys):
54+
hooked, triggered = False, False
55+
56+
if hasattr(sys, 'unraisablehook'): # Python 3.8+
57+
hooked = True
58+
default_hook = sys.unraisablehook
59+
60+
def hook(unraisable_hook_args):
61+
nonlocal triggered
62+
exc_type, exc_value, exc_tb, err_msg, obj = unraisable_hook_args
63+
if obj == 'PythonAlreadySetInDestructor':
64+
triggered = True
65+
default_hook(unraisable_hook_args)
66+
return
67+
68+
# Use monkeypatch so pytest can apply and remove the patch as appropriate
69+
monkeypatch.setattr(sys, 'unraisablehook', hook)
70+
71+
assert m.python_alreadyset_in_destructor('already_set demo') is True
72+
if hooked:
73+
assert triggered
74+
75+
captured = capsys.readouterr()
76+
assert captured.err.startswith(
77+
"Exception ignored in: 'PythonAlreadySetInDestructor'"
78+
)
79+
80+
5181
def test_exception_matches():
5282
assert m.exception_matches()
5383
assert m.exception_matches_base()

0 commit comments

Comments
 (0)