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 <pybind11/pybind11.h>
    #include <pybind11/stl_bind.h>
    namespace py = pybind11;
    PYBIND11_MODULE(z1, m) {
        py::bind_vector<std::vector<long>>(m, "IntVector");
    }

z2.cpp:
    #include <pybind11/pybind11.h>
    #include <pybind11/stl_bind.h>
    namespace py = pybind11;
    PYBIND11_MODULE(z2, m) {
        py::bind_vector<std::vector<double>>(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 <module>
        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++).
This commit is contained in:
Jason Rhinelander 2017-07-28 21:38:23 -04:00
parent 0bd5979c77
commit d598172993
4 changed files with 47 additions and 0 deletions

View File

@ -73,6 +73,23 @@ PYBIND11_NOINLINE inline internals &get_internals() {
const char *id = PYBIND11_INTERNALS_ID; const char *id = PYBIND11_INTERNALS_ID;
if (builtins.contains(id) && isinstance<capsule>(builtins[id])) { if (builtins.contains(id) && isinstance<capsule>(builtins[id])) {
internals_ptr = *static_cast<internals **>(capsule(builtins[id])); internals_ptr = *static_cast<internals **>(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 { } else {
internals_ptr = new internals(); internals_ptr = new internals();
#if defined(WITH_THREAD) #if defined(WITH_THREAD)

View File

@ -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 # 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. # doesn't include them) the second module doesn't get built.
set(PYBIND11_CROSS_MODULE_TESTS set(PYBIND11_CROSS_MODULE_TESTS
test_exceptions.py
) )
# Check if Eigen is available; if not, remove from PYBIND11_TEST_FILES (but # Check if Eigen is available; if not, remove from PYBIND11_TEST_FILES (but

View File

@ -17,4 +17,11 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) {
// Definitions here are tested by importing both this module and the // Definitions here are tested by importing both this module and the
// relevant pybind11_tests submodule from a test_whatever.py // 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(); });
} }

View File

@ -1,6 +1,7 @@
import pytest import pytest
from pybind11_tests import exceptions as m from pybind11_tests import exceptions as m
import pybind11_cross_module_tests as cm
def test_std_exception(msg): def test_std_exception(msg):
@ -19,6 +20,27 @@ def test_error_already_set(msg):
assert msg(excinfo.value) == "foo" 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(): def test_python_call_in_catch():
d = {} d = {}
assert m.python_call_in_destructor(d) is True assert m.python_call_in_destructor(d) is True