From 995433240c777199f2643bf1e53632314d3ebc6c Mon Sep 17 00:00:00 2001 From: Emery Conrad Date: Wed, 28 Oct 2026 10:55:21 -0500 Subject: [PATCH] [cpyrt] Stop the using-directive search at a cycle Mutual using-directives are valid C++ ([namespace.qual]/3). The attribute lookup recursed through them until the stack overflowed. Search each namespace at most once for each lookup. A lookup that Python code starts during the search gets its own search. Co-developed-with-the-help-of: Claude Code (Opus 5.5, human in the loop) --- src/cpyrt/CPPScope.cxx | 44 +++++++++++++++------ test/test_advancedcpp.py | 82 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 115 insertions(+), 11 deletions(-) diff --git a/src/cpyrt/CPPScope.cxx b/src/cpyrt/CPPScope.cxx index dd5801ab..6be765a2 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 6bd7aa7b..9d3a7355 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]