Skip to content

Commit f892cce

Browse files
committed
Use shared_ptr custom deleter for m_fetched_error in error_already_set. This enables removing the dtor, copy ctor, move ctor completely.
1 parent 4e06fb1 commit f892cce

File tree

2 files changed

+10
-21
lines changed

2 files changed

+10
-21
lines changed

include/pybind11/pybind11.h

+4-5
Original file line numberDiff line numberDiff line change
@@ -2625,13 +2625,12 @@ void print(Args &&...args) {
26252625
detail::print(c.args(), c.kwargs());
26262626
}
26272627

2628-
inline error_already_set::~error_already_set() {
2629-
if (m_fetched_error.use_count() != 1 || !m_fetched_error->has_py_object_references()) {
2630-
return; // Avoid gil and scope overhead if there is nothing to release.
2631-
}
2628+
inline void
2629+
error_already_set::m_fetched_error_deleter(detail::error_fetch_and_normalize *raw_ptr) {
26322630
gil_scoped_acquire gil;
26332631
error_scope scope;
2634-
m_fetched_error->release_py_object_references();
2632+
raw_ptr->release_py_object_references();
2633+
delete raw_ptr;
26352634
}
26362635

26372636
inline const char *error_already_set::what() const noexcept {

include/pybind11/pytypes.h

+6-16
Original file line numberDiff line numberDiff line change
@@ -541,8 +541,6 @@ struct error_fetch_and_normalize {
541541
return (PyErr_GivenExceptionMatches(m_type.ptr(), exc.ptr()) != 0);
542542
}
543543

544-
bool has_py_object_references() const { return m_type || m_value || m_trace; }
545-
546544
void release_py_object_references() {
547545
m_type.release().dec_ref();
548546
m_value.release().dec_ref();
@@ -580,20 +578,8 @@ class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::exception {
580578
/// Fetches the current Python exception (using PyErr_Fetch()), which will clear the
581579
/// current Python error indicator.
582580
error_already_set()
583-
: m_fetched_error{
584-
std::make_shared<detail::error_fetch_and_normalize>("pybind11::error_already_set")} {}
585-
586-
/// WARNING: This destructor needs to acquire the Python GIL. This can lead to
587-
/// crashes (undefined behavior) if the Python interpreter is finalizing.
588-
~error_already_set() override;
589-
590-
// This copy ctor does not need the GIL because it simply increments a shared_ptr use_count.
591-
error_already_set(const error_already_set &) noexcept = default;
592-
593-
// This move ctor cannot easily be deleted (some compilers need it).
594-
// It is the responsibility of the caller to not use the moved-from object.
595-
// For simplicity, guarding ifs are omitted.
596-
error_already_set(error_already_set &&) noexcept = default;
581+
: m_fetched_error{new detail::error_fetch_and_normalize("pybind11::error_already_set"),
582+
m_fetched_error_deleter} {}
597583

598584
/// The what() result is built lazily on demand.
599585
/// WARNING: This member function needs to acquire the Python GIL. This can lead to
@@ -637,6 +623,10 @@ class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::exception {
637623

638624
private:
639625
std::shared_ptr<detail::error_fetch_and_normalize> m_fetched_error;
626+
627+
/// WARNING: This custom deleter needs to acquire the Python GIL. This can lead to
628+
/// crashes (undefined behavior) if the Python interpreter is finalizing.
629+
static void m_fetched_error_deleter(detail::error_fetch_and_normalize *raw_ptr);
640630
};
641631
#if defined(_MSC_VER)
642632
# pragma warning(pop)

0 commit comments

Comments
 (0)