Skip to content

Commit

Permalink
Added exception translator specific mutex used with try_translate_exc…
Browse files Browse the repository at this point in the history
…eptions (#5362)

* Added exception translator specific mutex used with try_translate_exceptions
Fixes #5346

* - Replaced with_internals_for_exception_translator by with_exception_translators
- Incremented PYBIND11_INTERNALS_VERSION
- Added a test

* style: pre-commit fixes

* Fixed formatting and added explicit to ctors

* Addressed PR review comments

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
vfdev-5 and pre-commit-ci[bot] authored Sep 17, 2024
1 parent a7910be commit 1d9483f
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 21 deletions.
22 changes: 11 additions & 11 deletions include/pybind11/detail/exception_translation.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,17 @@ inline void try_translate_exceptions() {
- delegate translation to the next translator by throwing a new type of exception.
*/

bool handled = with_internals([&](internals &internals) {
auto &local_exception_translators = get_local_internals().registered_exception_translators;
if (detail::apply_exception_translators(local_exception_translators)) {
return true;
}
auto &exception_translators = internals.registered_exception_translators;
if (detail::apply_exception_translators(exception_translators)) {
return true;
}
return false;
});
bool handled = with_exception_translators(
[&](std::forward_list<ExceptionTranslator> &exception_translators,
std::forward_list<ExceptionTranslator> &local_exception_translators) {
if (detail::apply_exception_translators(local_exception_translators)) {
return true;
}
if (detail::apply_exception_translators(exception_translators)) {
return true;
}
return false;
});

if (!handled) {
set_error(PyExc_SystemError, "Exception escaped from default exception translator!");
Expand Down
20 changes: 19 additions & 1 deletion include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,11 @@
# if PY_VERSION_HEX >= 0x030C0000 || defined(_MSC_VER)
// Version bump for Python 3.12+, before first 3.12 beta release.
// Version bump for MSVC piggy-backed on PR #4779. See comments there.
# define PYBIND11_INTERNALS_VERSION 5
# ifdef Py_GIL_DISABLED
# define PYBIND11_INTERNALS_VERSION 6
# else
# define PYBIND11_INTERNALS_VERSION 5
# endif
# else
# define PYBIND11_INTERNALS_VERSION 4
# endif
Expand Down Expand Up @@ -177,6 +181,7 @@ static_assert(sizeof(instance_map_shard) % 64 == 0,
struct internals {
#ifdef Py_GIL_DISABLED
pymutex mutex;
pymutex exception_translator_mutex;
#endif
// std::type_index -> pybind11's type information
type_map<type_info *> registered_types_cpp;
Expand Down Expand Up @@ -643,6 +648,19 @@ inline auto with_internals(const F &cb) -> decltype(cb(get_internals())) {
return cb(internals);
}

template <typename F>
inline auto with_exception_translators(const F &cb)
-> decltype(cb(get_internals().registered_exception_translators,
get_local_internals().registered_exception_translators)) {
auto &internals = get_internals();
#ifdef Py_GIL_DISABLED
std::unique_lock<pymutex> lock((internals).exception_translator_mutex);
#endif
auto &local_internals = get_local_internals();
return cb(internals.registered_exception_translators,
local_internals.registered_exception_translators);
}

inline std::uint64_t mix64(std::uint64_t z) {
// David Stafford's variant 13 of the MurmurHash3 finalizer popularized
// by the SplitMix PRNG.
Expand Down
21 changes: 12 additions & 9 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -2586,10 +2586,12 @@ void implicitly_convertible() {
}

inline void register_exception_translator(ExceptionTranslator &&translator) {
detail::with_internals([&](detail::internals &internals) {
internals.registered_exception_translators.push_front(
std::forward<ExceptionTranslator>(translator));
});
detail::with_exception_translators(
[&](std::forward_list<ExceptionTranslator> &exception_translators,
std::forward_list<ExceptionTranslator> &local_exception_translators) {
(void) local_exception_translators;
exception_translators.push_front(std::forward<ExceptionTranslator>(translator));
});
}

/**
Expand All @@ -2599,11 +2601,12 @@ inline void register_exception_translator(ExceptionTranslator &&translator) {
* the exception.
*/
inline void register_local_exception_translator(ExceptionTranslator &&translator) {
detail::with_internals([&](detail::internals &internals) {
(void) internals;
detail::get_local_internals().registered_exception_translators.push_front(
std::forward<ExceptionTranslator>(translator));
});
detail::with_exception_translators(
[&](std::forward_list<ExceptionTranslator> &exception_translators,
std::forward_list<ExceptionTranslator> &local_exception_translators) {
(void) exception_translators;
local_exception_translators.push_front(std::forward<ExceptionTranslator>(translator));
});
}

/**
Expand Down
10 changes: 10 additions & 0 deletions tests/custom_exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from __future__ import annotations


class PythonMyException7(Exception):
def __init__(self, message):
self.message = message
super().__init__(message)

def __str__(self):
return "[PythonMyException7]: " + self.message.a
39 changes: 39 additions & 0 deletions tests/test_exceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,16 @@ struct PythonAlreadySetInDestructor {
py::str s;
};

struct CustomData {
explicit CustomData(const std::string &a) : a(a) {}
std::string a;
};

struct MyException7 {
explicit MyException7(const CustomData &message) : message(message) {}
CustomData message;
};

TEST_SUBMODULE(exceptions, m) {
m.def("throw_std_exception",
[]() { throw std::runtime_error("This exception was intentionally thrown."); });
Expand Down Expand Up @@ -385,4 +395,33 @@ TEST_SUBMODULE(exceptions, m) {

// m.def("pass_exception_void", [](const py::exception<void>&) {}); // Does not compile.
m.def("return_exception_void", []() { return py::exception<void>(); });

m.def("throws7", []() {
auto data = CustomData("abc");
throw MyException7(data);
});

py::class_<CustomData>(m, "CustomData", py::module_local())
.def(py::init<const std::string &>())
.def_readwrite("a", &CustomData::a);

PYBIND11_CONSTINIT static py::gil_safe_call_once_and_store<py::object>
PythonMyException7_storage;
PythonMyException7_storage.call_once_and_store_result([&]() {
auto mod = py::module_::import("custom_exceptions");
py::object obj = mod.attr("PythonMyException7");
return obj;
});

py::register_local_exception_translator([](std::exception_ptr p) {
try {
if (p) {
std::rethrow_exception(p);
}
} catch (const MyException7 &e) {
auto exc_type = PythonMyException7_storage.get_stored();
py::object exc_inst = exc_type(e.message);
PyErr_SetObject(PyExc_Exception, exc_inst.ptr());
}
});
}
5 changes: 5 additions & 0 deletions tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import sys

import pytest
from custom_exceptions import PythonMyException7

import env
import pybind11_cross_module_tests as cm
Expand Down Expand Up @@ -195,6 +196,10 @@ def test_custom(msg):
raise RuntimeError("Exception error: caught child from parent") from err
assert msg(excinfo.value) == "this is a helper-defined translated exception"

with pytest.raises(PythonMyException7) as excinfo:
m.throws7()
assert msg(excinfo.value) == "[PythonMyException7]: abc"


def test_nested_throws(capture):
"""Tests nested (e.g. C++ -> Python -> C++) exception handling"""
Expand Down

0 comments on commit 1d9483f

Please sign in to comment.