Skip to content

Commit

Permalink
Revert "error_already_set::what() is now constructed lazily (pybind#1895
Browse files Browse the repository at this point in the history 
)"

This reverts commit a05bc3d.

# Conflicts:
#	include/pybind11/pytypes.h
  • Loading branch information
@EricCousineau-TRI
EricCousineau-TRI committed Oct 13, 2023
1 parent 539fdbf commit cf59d90
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 264 deletions.
6 changes: 2 additions & 4 deletions include/pybind11/detail/class.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,8 +455,6 @@ extern "C" inline void pybind11_object_dealloc(PyObject *self) {
#endif
}

std::string error_string();

/** Create the type which can be used as a common base for all classes. This is
needed in order to satisfy Python's requirements for multiple inheritance.
Return value: New reference. */
Expand Down Expand Up @@ -492,7 +490,7 @@ inline PyObject *make_object_base_type(PyTypeObject *metaclass) {
type->tp_weaklistoffset = offsetof(instance, weakrefs);

if (PyType_Ready(type) < 0) {
pybind11_fail("PyType_Ready failed in make_object_base_type(): " + error_string());
pybind11_fail("PyType_Ready failed in make_object_base_type():" + error_string());
}

setattr((PyObject *) type, "__module__", str("pybind11_builtins"));
Expand Down Expand Up @@ -717,7 +715,7 @@ inline PyObject *make_new_python_type(const type_record &rec) {
}

if (PyType_Ready(type) < 0) {
pybind11_fail(std::string(rec.name) + ": PyType_Ready failed: " + error_string());
pybind11_fail(std::string(rec.name) + ": PyType_Ready failed (" + error_string() + ")!");
}

assert(!rec.dynamic_attr || PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC));
Expand Down
67 changes: 67 additions & 0 deletions include/pybind11/detail/type_caster_base.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,73 @@ PYBIND11_NOINLINE bool isinstance_generic(handle obj, const std::type_info &tp)
return isinstance(obj, type);
}

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.");
}

PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace);
if (scope.trace != nullptr) {
PyException_SetTraceback(scope.value, scope.trace);
}

std::string errorString;
if (scope.type) {
errorString += handle(scope.type).attr("__name__").cast<std::string>();
errorString += ": ";
}
if (scope.value) {
errorString += (std::string) str(scope.value);
}

#if !defined(PYPY_VERSION)
if (scope.trace) {
auto *trace = (PyTracebackObject *) scope.trace;

/* Get the deepest trace possible */
while (trace->tb_next) {
trace = trace->tb_next;
}

PyFrameObject *frame = trace->tb_frame;
Py_XINCREF(frame);
errorString += "\n\nAt:\n";
while (frame) {
# if PY_VERSION_HEX >= 0x030900B1
PyCodeObject *f_code = PyFrame_GetCode(frame);
# else
PyCodeObject *f_code = frame->f_code;
Py_INCREF(f_code);
# endif
int lineno = PyFrame_GetLineNumber(frame);
errorString += " ";
errorString += handle(f_code->co_filename).cast<std::string>();
errorString += '(';
errorString += std::to_string(lineno);
errorString += "): ";
errorString += handle(f_code->co_name).cast<std::string>();
errorString += '\n';
Py_DECREF(f_code);
# if PY_VERSION_HEX >= 0x030900B1
auto *b_frame = PyFrame_GetBack(frame);
# else
auto *b_frame = frame->f_back;
Py_XINCREF(b_frame);
# endif
Py_DECREF(frame);
frame = b_frame;
}
}
#endif

return errorString;
}

PYBIND11_NOINLINE handle get_object_handle(const void *ptr, const detail::type_info *type) {
auto &instances = get_internals().registered_instances;
auto range = instances.equal_range(ptr);
Expand Down
20 changes: 8 additions & 12 deletions include/pybind11/pybind11.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3043,21 +3043,17 @@ void print(Args &&...args) {
detail::print(c.args(), c.kwargs());
}

inline void
error_already_set::m_fetched_error_deleter(detail::error_fetch_and_normalize *raw_ptr) {
gil_scoped_acquire gil;
error_scope scope;
delete raw_ptr;
}

inline const char *error_already_set::what() const noexcept {
gil_scoped_acquire gil;
error_scope scope;
return m_fetched_error->error_string().c_str();
error_already_set::~error_already_set() {
if (m_type) {
gil_scoped_acquire gil;
error_scope scope;
m_type.release().dec_ref();
m_value.release().dec_ref();
m_trace.release().dec_ref();
}
}

PYBIND11_NAMESPACE_BEGIN(detail)

inline function
get_type_override(const void *this_ptr, const type_info *this_type, const char *name) {
handle self = get_object_handle(this_ptr, this_type);
Expand Down
224 changes: 32 additions & 192 deletions include/pybind11/pytypes.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -411,175 +411,7 @@ T reinterpret_steal(handle h) {
}

PYBIND11_NAMESPACE_BEGIN(detail)

// Equivalent to obj.__class__.__name__ (or obj.__name__ if obj is a class).
inline const char *obj_class_name(PyObject *obj) {
if (Py_TYPE(obj) == &PyType_Type) {
return reinterpret_cast<PyTypeObject *>(obj)->tp_name;
}
return Py_TYPE(obj)->tp_name;
}

std::string error_string();

struct error_fetch_and_normalize {
// Immediate normalization is long-established behavior (starting with
// https://github.com/pybind/pybind11/commit/135ba8deafb8bf64a15b24d1513899eb600e2011
// from Sep 2016) and safest. Normalization could be deferred, but this could mask
// errors elsewhere, the performance gain is very minor in typical situations
// (usually the dominant bottleneck is EH unwinding), and the implementation here
// would be more complex.
explicit error_fetch_and_normalize(const char *called) {
PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
if (!m_type) {
pybind11_fail("Internal error: " + std::string(called)
+ " called while "
"Python error indicator not set.");
}
const char *exc_type_name_orig = detail::obj_class_name(m_type.ptr());
if (exc_type_name_orig == nullptr) {
pybind11_fail("Internal error: " + std::string(called)
+ " failed to obtain the name "
"of the original active exception type.");
}
m_lazy_error_string = exc_type_name_orig;
// PyErr_NormalizeException() may change the exception type if there are cascading
// failures. This can potentially be extremely confusing.
PyErr_NormalizeException(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
if (m_type.ptr() == nullptr) {
pybind11_fail("Internal error: " + std::string(called)
+ " failed to normalize the "
"active exception.");
}
const char *exc_type_name_norm = detail::obj_class_name(m_type.ptr());
if (exc_type_name_orig == nullptr) {
pybind11_fail("Internal error: " + std::string(called)
+ " failed to obtain the name "
"of the normalized active exception type.");
}
if (exc_type_name_norm != m_lazy_error_string) {
std::string msg = std::string(called)
+ ": MISMATCH of original and normalized "
"active exception types: ";
msg += "ORIGINAL ";
msg += m_lazy_error_string;
msg += " REPLACED BY ";
msg += exc_type_name_norm;
msg += ": " + format_value_and_trace();
pybind11_fail(msg);
}
}

error_fetch_and_normalize(const error_fetch_and_normalize &) = delete;
error_fetch_and_normalize(error_fetch_and_normalize &&) = delete;

std::string format_value_and_trace() const {
std::string result;
std::string message_error_string;
if (m_value) {
auto value_str = reinterpret_steal<object>(PyObject_Str(m_value.ptr()));
if (!value_str) {
message_error_string = detail::error_string();
result = "<MESSAGE UNAVAILABLE DUE TO ANOTHER EXCEPTION>";
} else {
result = value_str.cast<std::string>();
}
} else {
result = "<MESSAGE UNAVAILABLE>";
}
if (result.empty()) {
result = "<EMPTY MESSAGE>";
}

bool have_trace = false;
if (m_trace) {
#if !defined(PYPY_VERSION)
auto *tb = reinterpret_cast<PyTracebackObject *>(m_trace.ptr());

// Get the deepest trace possible.
while (tb->tb_next) {
tb = tb->tb_next;
}

PyFrameObject *frame = tb->tb_frame;
Py_XINCREF(frame);
result += "\n\nAt:\n";
while (frame) {
# if PY_VERSION_HEX >= 0x030900B1
PyCodeObject *f_code = PyFrame_GetCode(frame);
# else
PyCodeObject *f_code = frame->f_code;
Py_INCREF(f_code);
# endif
int lineno = PyFrame_GetLineNumber(frame);
result += " ";
result += handle(f_code->co_filename).cast<std::string>();
result += '(';
result += std::to_string(lineno);
result += "): ";
result += handle(f_code->co_name).cast<std::string>();
result += '\n';
Py_DECREF(f_code);
# if PY_VERSION_HEX >= 0x030900B1
auto *b_frame = PyFrame_GetBack(frame);
# else
auto *b_frame = frame->f_back;
Py_XINCREF(b_frame);
# endif
Py_DECREF(frame);
frame = b_frame;
}

have_trace = true;
#endif //! defined(PYPY_VERSION)
}

if (!message_error_string.empty()) {
if (!have_trace) {
result += '\n';
}
result += "\nMESSAGE UNAVAILABLE DUE TO EXCEPTION: " + message_error_string;
}

return result;
}

std::string const &error_string() const {
if (!m_lazy_error_string_completed) {
m_lazy_error_string += ": " + format_value_and_trace();
m_lazy_error_string_completed = true;
}
return m_lazy_error_string;
}

void restore() {
if (m_restore_called) {
pybind11_fail("Internal error: pybind11::detail::error_fetch_and_normalize::restore() "
"called a second time. ORIGINAL ERROR: "
+ error_string());
}
PyErr_Restore(m_type.inc_ref().ptr(), m_value.inc_ref().ptr(), m_trace.inc_ref().ptr());
m_restore_called = true;
}

bool matches(handle exc) const {
return (PyErr_GivenExceptionMatches(m_type.ptr(), exc.ptr()) != 0);
}

// Not protecting these for simplicity.
object m_type, m_value, m_trace;

private:
// Only protecting invariants.
mutable std::string m_lazy_error_string;
mutable bool m_lazy_error_string_completed = false;
mutable bool m_restore_called = false;
};

inline std::string error_string() {
return error_fetch_and_normalize("pybind11::detail::error_string").error_string();
}

std::string error_string(const char *called = nullptr);
PYBIND11_NAMESPACE_END(detail)

#if defined(_MSC_VER)
Expand All @@ -592,30 +424,39 @@ PYBIND11_NAMESPACE_END(detail)
/// 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
/// python).
class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::exception {
class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::runtime_error {
public:
/// Fetches the current Python exception (using PyErr_Fetch()), which will clear the
/// current Python error indicator.
error_already_set()
: m_fetched_error{new detail::error_fetch_and_normalize("pybind11::error_already_set"),
m_fetched_error_deleter} {}

/// The what() result is built lazily on demand.
/// WARNING: This member function needs to acquire the Python GIL. This can lead to
/// 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("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.
const char *what() const noexcept override;
inline ~error_already_set() override;

/// Restores the currently-held Python error (which will clear the Python error indicator first
/// if already set).
/// 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() { m_fetched_error->restore(); }
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 a 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.
/// already knows the type and value of the error, so there is no need to repeat that. 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());
Expand All @@ -634,18 +475,16 @@ class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::exception {
/// Check if the currently trapped error type matches the given Python exception class (or a
/// subclass thereof). May also be passed a tuple to search for any exception class matches in
/// the given tuple.
bool matches(handle exc) const { return m_fetched_error->matches(exc); }
bool matches(handle exc) const {
return (PyErr_GivenExceptionMatches(m_type.ptr(), exc.ptr()) != 0);
}

const object &type() const { return m_fetched_error->m_type; }
const object &value() const { return m_fetched_error->m_value; }
const object &trace() const { return m_fetched_error->m_trace; }
const object &type() const { return m_type; }
const object &value() const { return m_value; }
const object &trace() const { return m_trace; }

private:
std::shared_ptr<detail::error_fetch_and_normalize> m_fetched_error;

/// WARNING: This custom deleter needs to acquire the Python GIL. This can lead to
/// crashes (undefined behavior) if the Python interpreter is finalizing.
static void m_fetched_error_deleter(detail::error_fetch_and_normalize *raw_ptr);
object m_type, m_value, m_trace;
};
#if defined(_MSC_VER)
# pragma warning(pop)
Expand Down Expand Up @@ -681,7 +520,8 @@ inline void raise_from(PyObject *type, const char *message) {

/// Sets the current Python error indicator with the chosen error, performing a 'raise from'
/// from the error contained in error_already_set to indicate that the chosen error was
/// caused by the original error.
/// caused by the original error. After this function is called error_already_set will
/// no longer contain an error.
inline void raise_from(error_already_set &err, PyObject *type, const char *message) {
err.restore();
raise_from(type, message);
Expand Down
Loading

0 comments on commit cf59d90

Please sign in to comment.