From d5981729934361f458d86fb9479b9ef68c26de9a Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Fri, 28 Jul 2017 21:38:23 -0400 Subject: [PATCH] Fix builtin exception handlers to work across modules The builtin exception handler currently doesn't work across modules under clang/libc++ for builtin pybind exceptions like `pybind11::error_already_set` or `pybind11::stop_iteration`: under RTLD_LOCAL module loading clang considers each module's exception classes distinct types. This then means that the base exception translator fails to catch the exceptions and the fall through to the generic `std::exception` handler, which completely breaks things like `stop_iteration`: only the `stop_iteration` of the first module loaded actually works properly; later modules raise a RuntimeError with no message when trying to invoke their iterators. For example, two modules defined like this exhibit the behaviour under clang++/libc++: z1.cpp: #include #include namespace py = pybind11; PYBIND11_MODULE(z1, m) { py::bind_vector>(m, "IntVector"); } z2.cpp: #include #include namespace py = pybind11; PYBIND11_MODULE(z2, m) { py::bind_vector>(m, "FloatVector"); } Python: import z1, z2 for i in z2.FloatVector(): pass results in: Traceback (most recent call last): File "zs.py", line 2, in for i in z2.FloatVector(): RuntimeError This commit fixes the issue by adding a new exception translator each time the internals pointer is initialized from python builtins: this generally means the internals data was initialized by some other module. (The extra translator(s) are skipped under libstdc++). --- include/pybind11/cast.h | 17 +++++++++++++++++ tests/CMakeLists.txt | 1 + tests/pybind11_cross_module_tests.cpp | 7 +++++++ tests/test_exceptions.py | 22 ++++++++++++++++++++++ 4 files changed, 47 insertions(+) diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index cb66e769e..0cee78609 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -73,6 +73,23 @@ PYBIND11_NOINLINE inline internals &get_internals() { const char *id = PYBIND11_INTERNALS_ID; if (builtins.contains(id) && isinstance(builtins[id])) { internals_ptr = *static_cast(capsule(builtins[id])); + + // We loaded builtins through python's builtins, which means that our error_already_set and + // builtin_exception may be different local classes than the ones set up in the initial + // exception translator, below, so add another for our local exception classes. + // + // stdlibc++ doesn't require this (types there are identified only by name) + #if !defined(__GLIBCXX__) + internals_ptr->registered_exception_translators.push_front( + [](std::exception_ptr p) -> void { + try { + if (p) std::rethrow_exception(p); + } catch (error_already_set &e) { e.restore(); return; + } catch (const builtin_exception &e) { e.set_error(); return; + } + } + ); + #endif } else { internals_ptr = new internals(); #if defined(WITH_THREAD) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bf0389005..8c7ca6388 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -71,6 +71,7 @@ string(REPLACE ".cpp" ".py" PYBIND11_PYTEST_FILES "${PYBIND11_TEST_FILES}") # built; if none of these are built (i.e. because TEST_OVERRIDE is used and # doesn't include them) the second module doesn't get built. set(PYBIND11_CROSS_MODULE_TESTS + test_exceptions.py ) # Check if Eigen is available; if not, remove from PYBIND11_TEST_FILES (but diff --git a/tests/pybind11_cross_module_tests.cpp b/tests/pybind11_cross_module_tests.cpp index c52bfbc3e..0053505a0 100644 --- a/tests/pybind11_cross_module_tests.cpp +++ b/tests/pybind11_cross_module_tests.cpp @@ -17,4 +17,11 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) { // Definitions here are tested by importing both this module and the // relevant pybind11_tests submodule from a test_whatever.py + // test_exceptions.py + m.def("raise_runtime_error", []() { PyErr_SetString(PyExc_RuntimeError, "My runtime error"); throw py::error_already_set(); }); + m.def("raise_value_error", []() { PyErr_SetString(PyExc_ValueError, "My value error"); throw py::error_already_set(); }); + m.def("throw_pybind_value_error", []() { throw py::value_error("pybind11 value error"); }); + m.def("throw_pybind_type_error", []() { throw py::type_error("pybind11 type error"); }); + m.def("throw_stop_iteration", []() { throw py::stop_iteration(); }); + } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 06d442e67..8d37c09b8 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -1,6 +1,7 @@ import pytest from pybind11_tests import exceptions as m +import pybind11_cross_module_tests as cm def test_std_exception(msg): @@ -19,6 +20,27 @@ def test_error_already_set(msg): assert msg(excinfo.value) == "foo" +def test_cross_module_exceptions(): + with pytest.raises(RuntimeError) as excinfo: + cm.raise_runtime_error() + assert str(excinfo.value) == "My runtime error" + + with pytest.raises(ValueError) as excinfo: + cm.raise_value_error() + assert str(excinfo.value) == "My value error" + + with pytest.raises(ValueError) as excinfo: + cm.throw_pybind_value_error() + assert str(excinfo.value) == "pybind11 value error" + + with pytest.raises(TypeError) as excinfo: + cm.throw_pybind_type_error() + assert str(excinfo.value) == "pybind11 type error" + + with pytest.raises(StopIteration) as excinfo: + cm.throw_stop_iteration() + + def test_python_call_in_catch(): d = {} assert m.python_call_in_destructor(d) is True