Fixing regression test for #517 so it fails on macos too

src/greenlet/tests/fail_c_stack_refs_suspended_gc.py

@@ -1,45 +1,35 @@
-"""Regression test for GC of a suspended greenlet's C-stack refs (issue #515).
+"""Regression test for issue #515: GC of a suspended greenlet's C-stack refs.
 
-When a greenlet suspends while the interpreter is holding a ``_PyCStackRef``
-(for example, mid attribute resolution), that deferred reference sits on the
-greenlet's C stack. The free-threaded collector only walks the running
-thread's list in ``gc_visit_thread_stacks``, so unless greenlet visits the
-suspended one from ``tp_traverse`` an object reachable only through it is freed
-early and used after free once the greenlet resumes.
+A greenlet that suspends mid attribute-resolution holds a ``_PyCStackRef`` to
+the looked-up object on its C stack. The free-threaded collector only walks the
+running thread's refs, so greenlet must visit a suspended greenlet's from
+``tp_traverse``, or an object reachable only through them is freed early.
 
-Reproducing that needs the pinned object to be deferred-refcounted (so its
-refcount doesn't keep it alive) and reachable only through the C-stack ref. A
-class is deferred-refcounted, and a metaclass ``__get__`` makes a class act as a
-descriptor -- so ``obj.attr``, where ``attr`` is such a class, pins the class in
-a ``_PyCStackRef`` across the (Python) ``__get__``. We switch away from inside
-``__get__``, drop every other reference to the class, and collect: on a
-regressed free-threaded build the class is gone by the time we resume; the fix
-keeps it alive because ``tp_traverse`` visited the snapshot taken at suspend.
-
-Prints "C STACK REFS GC OK" on a fixed build (and on with-GIL builds, where
-ordinary refcounting keeps the class alive); a regressed free-threaded build
-reports the class was collected and exits non-zero.
+The test pins a deferred-refcounted class (a metaclass ``__get__`` makes the
+class act as a descriptor), switches away from inside ``__get__``, drops every
+other reference, and collects. It passes only if the class is still alive and
+the suspended greenlet is what keeps it alive. Prints "C STACK REFS GC OK" and
+exits 0 on a fixed build, non-zero on a regressed one.
 """
 import gc
 import sys
+import sysconfig
 import weakref
 
 import greenlet
 
+# Only free-threaded builds have the C-stack-ref / deferred-refcount machinery.
+FREETHREAD = bool(sysconfig.get_config_var("Py_GIL_DISABLED"))
+
 parent = greenlet.getcurrent()
 observed = {}
 
 
 class Meta(type):
     def __get__(cls, obj, objtype=None):
-        # type(cls) is Meta, which defines __get__, so ``cls`` is a descriptor;
-        # the getattr machinery pins it in a _PyCStackRef across this call. A
-        # class is deferred-refcounted on a free-threaded build, so only that
-        # (deferred) C-stack ref will be keeping it alive in a moment.
-        #
-        # Drop the locals the descriptor protocol gave us (the class is ``cls``):
-        # greenlet visits a suspended greenlet's frames, so a leftover frame ref
-        # would keep the class alive on its own and mask the bug.
+        # ``cls`` is the class being resolved, pinned in a _PyCStackRef across
+        # this call. Drop the descriptor-protocol locals so a suspended frame
+        # can't keep the class alive on its own and mask the bug.
         del cls, obj, objtype
         child.switch()
         return 42
@@ -52,26 +42,44 @@ def __get__(cls, obj, objtype=None):
 
 
 def child_work():
-    # ``parent`` is suspended inside Meta.__get__, holding a C-stack ref to the
-    # class. Drop every *other* reference to it, then collect: now only greenlet
-    # visiting the suspended C-stack ref can keep the class alive.
+    # parent is suspended inside Meta.__get__ holding a C-stack ref to the class.
+    # Drop every other reference, then collect.
     ref = weakref.ref(box[0])
     box[0] = None
     del holder.attr
     for _ in range(5):
         gc.collect()
-    observed['alive'] = ref() is not None
+    cls = ref()
+    if cls is None:
+        observed['status'] = 'collected'
+    elif not FREETHREAD or any(r is parent for r in gc.get_referrers(cls)):
+        # Kept alive by the suspended greenlet (or, with the GIL, by refcounting).
+        observed['status'] = 'ok'
+    else:
+        # Alive, but not because of the greenlet: a masking reference hid the
+        # C-stack-ref path (the class's own mro/bases are a self-cycle, not one).
+        observed['status'] = 'masked'
+        own = (cls.__mro__, cls.__bases__)
+        observed['maskers'] = sorted(
+            {type(r).__name__ for r in gc.get_referrers(cls)
+             if r is not parent and r is not own[0] and r is not own[1]}
+        ) or ['<invisible to gc; strong C-stack ref / non-deferred class>']
+    del cls
     parent.switch()
 
 
 child = greenlet.greenlet(child_work)
-result = holder().attr                             # Meta.__get__ -> switch -> gc -> back
+result = holder().attr
 assert result == 42, result
 
-alive = observed.get('alive')
+status = observed.get('status')
 print(f"py={sys.version.split()[0]} gil={getattr(sys, '_is_gil_enabled', lambda: True)()} "
-      f"greenlet={greenlet.__version__} alive={alive}", flush=True)
-if not alive:
-    raise SystemExit("REGRESSED: a class reachable only through a suspended "
+      f"greenlet={greenlet.__version__} status={status}", flush=True)
+if status == 'collected':
+    raise SystemExit("REGRESSED: class reachable only through a suspended "
                      "greenlet's C-stack ref was collected early")
+if status == 'masked':
+    raise SystemExit("REGRESSED: class stayed alive but not through the suspended "
+                     "greenlet; a masking reference (%s) hid the C-stack-ref path"
+                     % ', '.join(observed.get('maskers', ())))
 print("C STACK REFS GC OK", flush=True)

src/greenlet/tests/test_gc.py

@@ -97,15 +97,10 @@ def test_issue515_freethread_c_stack_refs(self):
         self.assertIn('ISSUE 515 OK', output)
 
     def test_c_stack_refs_suspended_gc(self):
-        # Review follow-up to #515: a greenlet that suspends while holding a
-        # _PyCStackRef has those deferred references visited by tp_traverse (the
-        # snapshot in TPythonState.cpp), so the free-threaded collector can't
-        # free an object reachable only through the suspended greenlet's C stack.
-        # The script pins a (deferred-refcounted) class via a metaclass __get__,
-        # switches away from inside it, drops every other reference, and
-        # collects; without the fix the class is gone on resume. Out of process
-        # because the regression is a use-after-free.
-        # https://github.com/python-greenlet/greenlet/issues/515
+        # Issue #515: a greenlet suspended while holding a _PyCStackRef must have
+        # those refs visited by tp_traverse, or the free-threaded collector frees
+        # an object reachable only through the suspended C stack. Runs the repro
+        # out of process. https://github.com/python-greenlet/greenlet/issues/515
         if not RUNNING_ON_FREETHREAD_BUILD:
             self.skipTest("Only free-threaded builds are affected")
         output = self.run_script('fail_c_stack_refs_suspended_gc.py')