diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 14561759b6..dd8d03751e 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -470,14 +470,16 @@ PYBIND11_NOINLINE bool isinstance_generic(handle obj, const std::type_info &tp) return isinstance(obj, type); } -PYBIND11_NOINLINE std::string error_string() { - if (!PyErr_Occurred()) { - PyErr_SetString(PyExc_RuntimeError, "Unknown internal error occurred"); - return "Unknown internal error occurred"; +PYBIND11_NOINLINE std::string error_string(const char *called) { + error_scope scope; // Fetch error state (will be restored when this function returns). + if (scope.type == nullptr) { + if (called == nullptr) { + called = "pybind11::detail::error_string()"; + } + pybind11_fail("Internal error: " + std::string(called) + + " called while Python error indicator not set."); } - error_scope scope; // Preserve error state - PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace); if (scope.trace != nullptr) { PyException_SetTraceback(scope.value, scope.trace); diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 5b1c60e337..c71f1b2e90 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -360,7 +360,7 @@ T reinterpret_steal(handle h) { } PYBIND11_NAMESPACE_BEGIN(detail) -std::string error_string(); +std::string error_string(const char *called = nullptr); PYBIND11_NAMESPACE_END(detail) #if defined(_MSC_VER) @@ -375,20 +375,27 @@ PYBIND11_NAMESPACE_END(detail) /// python). class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::runtime_error { public: - /// Constructs a new exception from the current Python error indicator, if any. The current + /// Constructs a new exception from the current Python error indicator. The current /// Python error indicator will be cleared. - error_already_set() : std::runtime_error(detail::error_string()) { + error_already_set() : std::runtime_error(detail::error_string("pybind11::error_already_set")) { PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr()); } + /// WARNING: The GIL must be held when this copy constructor is invoked! error_already_set(const error_already_set &) = default; error_already_set(error_already_set &&) = default; + /// WARNING: This destructor needs to acquire the Python GIL. This can lead to + /// crashes (undefined behavior) if the Python interpreter is finalizing. inline ~error_already_set() override; - /// Give the currently-held error back to Python, if any. If there is currently a Python error - /// already set it is cleared first. After this call, the current object no longer stores the - /// error variables (but the `.what()` string is still available). + /// Restores the currently-held Python error (which will clear the Python error indicator first + /// if already set). After this call, the current object no longer stores the error variables. + /// NOTE: Any copies of this object may still store the error variables. Currently there is no + // protection against calling restore() from multiple copies. + /// NOTE: This member function will always restore the normalized exception, which may or may + /// not be the original Python exception. + /// WARNING: The GIL must be held when this member function is called! void restore() { PyErr_Restore(m_type.release().ptr(), m_value.release().ptr(), m_trace.release().ptr()); } @@ -405,6 +412,7 @@ class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::runtime_error { } /// An alternate version of `discard_as_unraisable()`, where a string provides information on /// the location of the error. For example, `__func__` could be helpful. + /// WARNING: The GIL must be held when this member function is called! void discard_as_unraisable(const char *err_context) { discard_as_unraisable(reinterpret_steal(PYBIND11_FROM_STRING(err_context))); } diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index d7b31cf744..163fcd731a 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -228,7 +228,10 @@ TEST_SUBMODULE(exceptions, m) { throw py::error_already_set(); } catch (const std::runtime_error &e) { if ((err && e.what() != std::string("ValueError: foo")) - || (!err && e.what() != std::string("Unknown internal error occurred"))) { + || (!err + && e.what() + != std::string("Internal error: pybind11::error_already_set called " + "while Python error indicator not set."))) { PyErr_Clear(); throw std::runtime_error("error message mismatch"); } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index dc75941134..361bb800cb 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -16,7 +16,10 @@ def test_std_exception(msg): def test_error_already_set(msg): with pytest.raises(RuntimeError) as excinfo: m.throw_already_set(False) - assert msg(excinfo.value) == "Unknown internal error occurred" + assert ( + msg(excinfo.value) + == "Internal error: pybind11::error_already_set called while Python error indicator not set." + ) with pytest.raises(ValueError) as excinfo: m.throw_already_set(True)