diff --git a/src/cpyrt/CPPScope.cxx b/src/cpyrt/CPPScope.cxx index dd5801a..6be765a 100644 --- a/src/cpyrt/CPPScope.cxx +++ b/src/cpyrt/CPPScope.cxx @@ -320,7 +320,18 @@ static PyObject* pt_new(PyTypeObject* subtype, PyObject* args, PyObject* kwds) { } //---------------------------------------------------------------------------- +// The (scope, name) pairs that one lookup has searched through using +// directives ([namespace.qual]/3), so that using-directive cycles terminate. +using UsingSearch = std::set>; +static thread_local UsingSearch* tUsingSearch = nullptr; +// Set by the using-directive loop for the scope lookup that it starts next. +static thread_local bool tUsingHop = false; + static PyObject* meta_getattro(PyObject* pyclass, PyObject* pyname) { + // read before the type lookup below can run Python code + const bool using_hop = tUsingHop; + tUsingHop = false; + // normal type-based lookup PyObject* attr = PyType_Type.tp_getattro(pyclass, pyname); if (pyclass == (PyObject*)&CPPInstance_Type) @@ -505,19 +516,30 @@ static PyObject* meta_getattro(PyObject* pyclass, PyObject* pyname) { } } - // try all outstanding using namespaces in turn to find the attribute (will - // cache locally later; TODO: doing so may cause pathological cases) - for (auto pyref : *klass->fImp.fUsing) { - PyObject* pyuscope = cpyrt_GetWeakRef(pyref); - if (pyuscope) { - attr = PyObject_GetAttr(pyuscope, pyname); - if (!attr) - PyErr_Clear(); - Py_DECREF(pyuscope); + // A hop from the loop below continues its lookup's search; any other + // lookup, e.g. from Python code that runs during the loop, starts afresh. + UsingSearch fresh; + UsingSearch* const outer = tUsingSearch; + if (!using_hop || !outer) + tUsingSearch = &fresh; + if (tUsingSearch->emplace(pyclass, name).second) { + // try all outstanding using namespaces in turn to find the attribute + // (will cache locally later; TODO: doing so may cause pathological cases) + for (auto pyref : *klass->fImp.fUsing) { + PyObject* pyuscope = cpyrt_GetWeakRef(pyref); + if (pyuscope) { + tUsingHop = true; + attr = PyObject_GetAttr(pyuscope, pyname); + tUsingHop = false; + if (!attr) + PyErr_Clear(); + Py_DECREF(pyuscope); + } + if (attr) + break; } - if (attr) - break; } + tUsingSearch = outer; } // if the attribute was not found but could possibly have been shadowed, diff --git a/test/test_advancedcpp.py b/test/test_advancedcpp.py index 6bd7aa7..9d3a735 100644 --- a/test/test_advancedcpp.py +++ b/test/test_advancedcpp.py @@ -1098,3 +1098,85 @@ def test31_virtual_base_downcast(self): assert isinstance(obj3, ns.Visible3) assert obj3.value == 23 assert obj3.z == 7 + + def test32_using_directive_cycle(self): + """Lookup through cyclic using directives searches each namespace once""" + + import cppjit + + cppjit.cppdef("""\ + namespace udir_cycle_a {} + namespace udir_cycle_p { using namespace udir_cycle_a; int x = 1; } + namespace udir_cycle_a { using namespace udir_cycle_p; int y = 2; }""") + + a = cppjit.gbl.udir_cycle_a + p = cppjit.gbl.udir_cycle_p + assert a.x == 1 + assert p.y == 2 + + with raises(AttributeError): + a.nothere + with raises(AttributeError): + p.nothere + + def test33_using_directive_lookup_from_other_thread(self): + """A lookup from another thread does not see a running lookup's search""" + + import threading + + import cppjit + + cppjit.cppdef("""\ + namespace udir_thread_a {} + namespace udir_thread_p { using namespace udir_thread_a; } + namespace udir_thread_a { using namespace udir_thread_p; }""") + + a = cppjit.gbl.udir_thread_a + p = cppjit.gbl.udir_thread_p + started, inner = [], [] + + def lookup(): + try: + inner.append(a.hook) + except AttributeError: + inner.append(None) + + class Hook: + # udir_thread_a.hook resolves only through the using directive, so + # __get__ runs while that lookup's search is still open + def __get__(self, obj, owner): + if not started: + started.append(True) + t = threading.Thread(target=lookup) + t.start() + t.join() # releases the GIL + return 42 + + p.hook = Hook() + assert a.hook == 42 + assert inner == [42] + + def test34_using_directive_lookup_reentered(self): + """A lookup from Python code that runs during a lookup searches afresh""" + + import cppjit + + cppjit.cppdef("""\ + namespace udir_reenter_a {} + namespace udir_reenter_p { using namespace udir_reenter_a; } + namespace udir_reenter_a { using namespace udir_reenter_p; }""") + + a = cppjit.gbl.udir_reenter_a + p = cppjit.gbl.udir_reenter_p + inner = [] + + class Hook: + def __get__(self, obj, owner): + if not inner: + inner.append(None) + inner[0] = a.hook + return 42 + + p.hook = Hook() + assert a.hook == 42 + assert inner == [42]