From 8ed5b8ab55130f8fcea24d3e90fc3a75886fd3c9 Mon Sep 17 00:00:00 2001 From: Wenzel Jakob Date: Mon, 28 Aug 2017 16:34:06 +0200 Subject: [PATCH] make implicit conversions non-reentrant (fixes #1035) (#1037) --- docs/advanced/classes.rst | 4 ++++ include/pybind11/pybind11.h | 9 +++++++++ tests/test_class.cpp | 11 +++++++++++ tests/test_class.py | 10 ++++++++++ 4 files changed, 34 insertions(+) diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index 8926fa928..7bcd0385a 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -585,6 +585,10 @@ Python side: Implicit conversions from ``A`` to ``B`` only work when ``B`` is a custom data type that is exposed to Python via pybind11. + To prevent runaway recursion, implicit conversions are non-reentrant: an + implicit conversion invoked as part of another implicit conversion of the + same type (i.e. from ``A`` to ``B``) will fail. + .. _static_properties: Static properties diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 9ed5036d6..16e3fdc3e 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1536,7 +1536,16 @@ template void implicitly_convertible() { + struct set_flag { + bool &flag; + set_flag(bool &flag) : flag(flag) { flag = true; } + ~set_flag() { flag = false; } + }; auto implicit_caster = [](PyObject *obj, PyTypeObject *type) -> PyObject * { + static bool currently_used = false; + if (currently_used) // implicit conversions are non-reentrant + return nullptr; + set_flag flag_helper(currently_used); if (!detail::make_caster().load(obj, false)) return nullptr; tuple args(1); diff --git a/tests/test_class.cpp b/tests/test_class.cpp index 842991aaf..222190617 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -291,6 +291,17 @@ TEST_SUBMODULE(class_, m) { .def(py::init()) .def_readwrite("field1", &BraceInitialization::field1) .def_readwrite("field2", &BraceInitialization::field2); + + // test_reentrant_implicit_conversion_failure + // #1035: issue with runaway reentrant implicit conversion + struct BogusImplicitConversion { + BogusImplicitConversion(const BogusImplicitConversion &) { } + }; + + py::class_(m, "BogusImplicitConversion") + .def(py::init()); + + py::implicitly_convertible(); } template class BreaksBase { public: virtual ~BreaksBase() = default; }; diff --git a/tests/test_class.py b/tests/test_class.py index ac2a3c0e3..412d6798e 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -223,3 +223,13 @@ def test_class_refcount(): assert refcount_1 == refcount_3 assert refcount_2 > refcount_1 + + +def test_reentrant_implicit_conversion_failure(msg): + # ensure that there is no runaway reentrant implicit conversion (#1035) + with pytest.raises(TypeError) as excinfo: + m.BogusImplicitConversion(0) + assert msg(excinfo.value) == '''__init__(): incompatible constructor arguments. The following argument types are supported: + 1. m.class_.BogusImplicitConversion(arg0: m.class_.BogusImplicitConversion) + +Invoked with: 0'''