error_already_set::what() is now constructed lazily

Prior to this commit throwing error_already_set was expensive due to the
eager construction of the error string (which required traversing the
Python stack). See #1853 for more context and an alternative take on the
issue.

Note that error_already_set no longer inherits from std::runtime_error
because the latter has no default constructor.
This commit is contained in:
Sergei Lebedev 2019-08-27 10:50:47 +01:00
parent 12e8774bc9
commit 87d1f6ba08
3 changed files with 40 additions and 22 deletions

View File

@ -403,38 +403,36 @@ PYBIND11_NOINLINE inline bool isinstance_generic(handle obj, const std::type_inf
return isinstance(obj, type); return isinstance(obj, type);
} }
PYBIND11_NOINLINE inline std::string error_string() { PYBIND11_NOINLINE inline std::string error_string(PyObject* type, PyObject* value, PyObject *trace) {
if (!PyErr_Occurred()) { if (!type && !value && !trace) {
PyErr_SetString(PyExc_RuntimeError, "Unknown internal error occurred"); PyErr_SetString(PyExc_RuntimeError, "Unknown internal error occurred");
return "Unknown internal error occurred"; return "Unknown internal error occurred";
} }
error_scope scope; // Preserve error state // TODO(superbobry): is it safe to assume that exception has been
// normalized by the caller?
std::string errorString; std::string errorString;
if (scope.type) { if (type) {
errorString += handle(scope.type).attr("__name__").cast<std::string>(); errorString += handle(type).attr("__name__").cast<std::string>();
errorString += ": "; errorString += ": ";
} }
if (scope.value) if (value)
errorString += (std::string) str(scope.value); errorString += str(value).cast<std::string>();
PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace);
#if PY_MAJOR_VERSION >= 3 #if PY_MAJOR_VERSION >= 3
if (scope.trace != nullptr) if (trace)
PyException_SetTraceback(scope.value, scope.trace); PyException_SetTraceback(value, trace);
#endif #endif
#if !defined(PYPY_VERSION) #if !defined(PYPY_VERSION)
if (scope.trace) { if (trace) {
PyTracebackObject *trace = (PyTracebackObject *) scope.trace; PyTracebackObject *tb = (PyTracebackObject *) trace;
/* Get the deepest trace possible */ /* Get the deepest trace possible */
while (trace->tb_next) while (tb->tb_next)
trace = trace->tb_next; tb = tb->tb_next;
PyFrameObject *frame = trace->tb_frame; PyFrameObject *frame = tb->tb_frame;
errorString += "\n\nAt:\n"; errorString += "\n\nAt:\n";
while (frame) { while (frame) {
int lineno = PyFrame_GetLineNumber(frame); int lineno = PyFrame_GetLineNumber(frame);
@ -450,6 +448,12 @@ PYBIND11_NOINLINE inline std::string error_string() {
return errorString; return errorString;
} }
PYBIND11_NOINLINE inline std::string error_string() {
error_scope scope; // Preserve error state
PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace);
return error_string(scope.type, scope.value, scope.trace);
}
PYBIND11_NOINLINE inline handle get_object_handle(const void *ptr, const detail::type_info *type ) { PYBIND11_NOINLINE inline handle get_object_handle(const void *ptr, const detail::type_info *type ) {
auto &instances = get_internals().registered_instances; auto &instances = get_internals().registered_instances;
auto range = instances.equal_range(ptr); auto range = instances.equal_range(ptr);

View File

@ -313,17 +313,18 @@ template <typename T> T reinterpret_steal(handle h) { return {h, object::stolen_
NAMESPACE_BEGIN(detail) NAMESPACE_BEGIN(detail)
inline std::string error_string(); inline std::string error_string();
inline std::string error_string(PyObject*, PyObject*, PyObject*);
NAMESPACE_END(detail) NAMESPACE_END(detail)
/// Fetch and hold an error which was already set in Python. An instance of this is typically /// Fetch and hold an error which was already set in Python. An instance of this is typically
/// thrown to propagate python-side errors back through C++ which can either be caught manually or /// thrown to propagate python-side errors back through C++ which can either be caught manually or
/// else falls back to the function dispatcher (which then raises the captured error back to /// else falls back to the function dispatcher (which then raises the captured error back to
/// python). /// python).
class error_already_set : public std::runtime_error { class error_already_set : public std::exception {
public: 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, if any. The current
/// Python error indicator will be cleared. /// Python error indicator will be cleared.
error_already_set() : std::runtime_error(detail::error_string()) { error_already_set() : std::exception() {
PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr()); PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
} }
@ -332,10 +333,22 @@ public:
inline ~error_already_set(); inline ~error_already_set();
virtual const char* what() const noexcept {
if (m_lazy_what.empty()) {
PyErr_NormalizeException(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
m_lazy_what = detail::error_string(m_type.ptr(), m_value.ptr(), m_trace.ptr());
}
return m_lazy_what.c_str();
}
/// Give the currently-held error back to Python, if any. If there is currently a Python error /// 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 /// 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). /// 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()); } void restore() {
what(); // Force-build `.what()`.
if (m_type || m_value || m_trace)
PyErr_Restore(m_type.release().ptr(), m_value.release().ptr(), m_trace.release().ptr());
}
// Does nothing; provided for backwards compatibility. // Does nothing; provided for backwards compatibility.
PYBIND11_DEPRECATED("Use of error_already_set.clear() is deprecated") PYBIND11_DEPRECATED("Use of error_already_set.clear() is deprecated")
@ -351,7 +364,8 @@ public:
const object& trace() const { return m_trace; } const object& trace() const { return m_trace; }
private: private:
object m_type, m_value, m_trace; mutable std::string m_lazy_what;
mutable object m_type, m_value, m_trace;
}; };
/** \defgroup python_builtins _ /** \defgroup python_builtins _

View File

@ -157,7 +157,7 @@ TEST_SUBMODULE(exceptions, m) {
PyErr_SetString(PyExc_ValueError, "foo"); PyErr_SetString(PyExc_ValueError, "foo");
try { try {
throw py::error_already_set(); throw py::error_already_set();
} catch (const std::runtime_error& e) { } catch (const py::error_already_set& e) {
if ((err && e.what() != std::string("ValueError: foo")) || if ((err && e.what() != std::string("ValueError: foo")) ||
(!err && e.what() != std::string("Unknown internal error occurred"))) (!err && e.what() != std::string("Unknown internal error occurred")))
{ {