gh-125900: Clean-up logic around immortalization in free-threading (#125901)

colesbury · web-flow · commit 332356b88057 · 2024-10-24T18:09:59.000-04:00
* Remove `@suppress_immortalization` decorator
* Make suppression flag per-thread instead of per-interpreter
* Suppress immortalization in `eval()` to avoid refleaks in three tests
  (test_datetime.test_roundtrip, test_logging.test_config8_ok, and
   test_random.test_after_fork).
* frozenset() is constant, but not a singleton. When run multiple times,
  the test could fail due to constant interning.
diff --git a/Include/internal/pycore_gc.h b/Include/internal/pycore_gc.h
@@ -342,14 +342,6 @@ struct _gc_runtime_state {
        collections, and are awaiting to undergo a full collection for
        the first time. */
     Py_ssize_t long_lived_pending;
-
-    /* gh-117783: Deferred reference counting is not fully implemented yet, so
-       as a temporary measure we treat objects using deferred reference
-       counting as immortal. The value may be zero, one, or a negative number:
-        0: immortalize deferred RC objects once the first thread is created
-        1: immortalize all deferred RC objects immediately
-        <0: suppressed; don't immortalize objects */
-    int immortalize;
 #endif
 };
 
diff --git a/Include/internal/pycore_tstate.h b/Include/internal/pycore_tstate.h
@@ -41,6 +41,9 @@ typedef struct _PyThreadStateImpl {
         // If set, don't use per-thread refcounts
         int is_finalized;
     } refcounts;
+
+    // When >1, code objects do not immortalize their non-string constants.
+    int suppress_co_const_immortalization;
 #endif
 
 #if defined(Py_REF_DEBUG) && defined(Py_GIL_DISABLED)
diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py
@@ -7,8 +7,7 @@
 import time
 import trace
 
-from test.support import (os_helper, MS_WINDOWS, flush_std_streams,
-                          suppress_immortalization)
+from test.support import os_helper, MS_WINDOWS, flush_std_streams
 
 from .cmdline import _parse_args, Namespace
 from .findtests import findtests, split_test_packages, list_cases
@@ -535,10 +534,7 @@ def _run_tests(self, selected: TestTuple, tests: TestList | None) -> int:
             if self.num_workers:
                 self._run_tests_mp(runtests, self.num_workers)
             else:
-                # gh-117783: don't immortalize deferred objects when tracking
-                # refleaks. Only relevant for the free-threaded build.
-                with suppress_immortalization(runtests.hunt_refleak):
-                    self.run_tests_sequentially(runtests)
+                self.run_tests_sequentially(runtests)
 
             coverage = self.results.get_coverage_results()
             self.display_result(runtests)
diff --git a/Lib/test/libregrtest/single.py b/Lib/test/libregrtest/single.py
@@ -304,10 +304,7 @@ def run_single_test(test_name: TestName, runtests: RunTests) -> TestResult:
     result = TestResult(test_name)
     pgo = runtests.pgo
     try:
-        # gh-117783: don't immortalize deferred objects when tracking
-        # refleaks. Only relevant for the free-threaded build.
-        with support.suppress_immortalization(runtests.hunt_refleak):
-            _runtest(result, runtests)
+        _runtest(result, runtests)
     except:
         if not pgo:
             msg = traceback.format_exc()
diff --git a/Lib/test/seq_tests.py b/Lib/test/seq_tests.py
@@ -426,7 +426,6 @@ def test_pickle(self):
             self.assertEqual(lst2, lst)
             self.assertNotEqual(id(lst2), id(lst))
 
-    @support.suppress_immortalization()
     def test_free_after_iterating(self):
         support.check_free_after_iterating(self, iter, self.type2test)
         support.check_free_after_iterating(self, reversed, self.type2test)
diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py
@@ -512,33 +512,6 @@ def has_no_debug_ranges():
 def requires_debug_ranges(reason='requires co_positions / debug_ranges'):
     return unittest.skipIf(has_no_debug_ranges(), reason)
 
-@contextlib.contextmanager
-def suppress_immortalization(suppress=True):
-    """Suppress immortalization of deferred objects."""
-    try:
-        import _testinternalcapi
-    except ImportError:
-        yield
-        return
-
-    if not suppress:
-        yield
-        return
-
-    _testinternalcapi.suppress_immortalization(True)
-    try:
-        yield
-    finally:
-        _testinternalcapi.suppress_immortalization(False)
-
-def skip_if_suppress_immortalization():
-    try:
-        import _testinternalcapi
-    except ImportError:
-        return
-    return unittest.skipUnless(_testinternalcapi.get_immortalize_deferred(),
-                                "requires immortalization of deferred objects")
-
 
 MS_WINDOWS = (sys.platform == 'win32')
 
diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py
@@ -2259,7 +2259,7 @@ def test_validation(self):
                          "got an invalid type in Constant: list")
 
     def test_singletons(self):
-        for const in (None, False, True, Ellipsis, b'', frozenset()):
+        for const in (None, False, True, Ellipsis, b''):
             with self.subTest(const=const):
                 value = self.compile_constant(const)
                 self.assertIs(value, const)
diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py
@@ -25,7 +25,6 @@
 from test.support import threading_helper
 from test.support import warnings_helper
 from test.support import requires_limited_api
-from test.support import suppress_immortalization
 from test.support import expected_failure_if_gil_disabled
 from test.support import Py_GIL_DISABLED
 from test.support.script_helper import assert_python_failure, assert_python_ok, run_python_until_end
@@ -481,7 +480,6 @@ def test_heap_ctype_doc_and_text_signature(self):
     def test_null_type_doc(self):
         self.assertEqual(_testcapi.NullTpDocType.__doc__, None)
 
-    @suppress_immortalization()
     def test_subclass_of_heap_gc_ctype_with_tpdealloc_decrefs_once(self):
         class HeapGcCTypeSubclass(_testcapi.HeapGcCType):
             def __init__(self):
@@ -499,7 +497,6 @@ def __init__(self):
         del subclass_instance
         self.assertEqual(type_refcnt - 1, sys.getrefcount(HeapGcCTypeSubclass))
 
-    @suppress_immortalization()
     def test_subclass_of_heap_gc_ctype_with_del_modifying_dunder_class_only_decrefs_once(self):
         class A(_testcapi.HeapGcCType):
             def __init__(self):
diff --git a/Lib/test/test_capi/test_watchers.py b/Lib/test/test_capi/test_watchers.py
@@ -4,7 +4,7 @@
 from contextlib import contextmanager, ExitStack
 from test.support import (
     catch_unraisable_exception, import_helper,
-    gc_collect, suppress_immortalization)
+    gc_collect)
 
 
 # Skip this test if the _testcapi module isn't available.
@@ -404,7 +404,6 @@ def assert_event_counts(self, exp_created_0, exp_destroyed_0,
         self.assertEqual(
             exp_destroyed_1, _testcapi.get_code_watcher_num_destroyed_events(1))
 
-    @suppress_immortalization()
     def test_code_object_events_dispatched(self):
         # verify that all counts are zero before any watchers are registered
         self.assert_event_counts(0, 0, 0, 0)
@@ -451,7 +450,6 @@ def test_error(self):
                 self.assertIsNone(cm.unraisable.object)
                 self.assertEqual(str(cm.unraisable.exc_value), "boom!")
 
-    @suppress_immortalization()
     def test_dealloc_error(self):
         co = _testcapi.code_newempty("test_watchers", "dummy0", 0)
         with self.code_watcher(2):
diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py
@@ -141,9 +141,7 @@
     ctypes = None
 from test.support import (cpython_only,
                           check_impl_detail, requires_debug_ranges,
-                          gc_collect, Py_GIL_DISABLED,
-                          suppress_immortalization,
-                          skip_if_suppress_immortalization)
+                          gc_collect, Py_GIL_DISABLED)
 from test.support.script_helper import assert_python_ok
 from test.support import threading_helper, import_helper
 from test.support.bytecode_helper import instructions_with_positions
@@ -579,7 +577,6 @@ def test_interned_string_with_null(self):
 
     @cpython_only
     @unittest.skipUnless(Py_GIL_DISABLED, "does not intern all constants")
-    @skip_if_suppress_immortalization()
     def test_interned_constants(self):
         # compile separately to avoid compile time de-duping
 
@@ -599,7 +596,6 @@ def func2():
 
 class CodeWeakRefTest(unittest.TestCase):
 
-    @suppress_immortalization()
     def test_basic(self):
         # Create a code object in a clean environment so that we know we have
         # the only reference to it left.
@@ -850,7 +846,6 @@ def test_bad_index(self):
             self.assertEqual(GetExtra(f.__code__, FREE_INDEX+100,
                               ctypes.c_voidp(100)), 0)
 
-        @suppress_immortalization()
         def test_free_called(self):
             # Verify that the provided free function gets invoked
             # when the code object is cleaned up.
@@ -878,7 +873,6 @@ def test_get_set(self):
             del f
 
         @threading_helper.requires_working_threading()
-        @suppress_immortalization()
         def test_free_different_thread(self):
             # Freeing a code object on a different thread then
             # where the co_extra was set should be safe.
diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
@@ -5076,7 +5076,6 @@ def __new__(cls):
                 cls.lst = [2**i for i in range(10000)]
         X.descr
 
-    @support.suppress_immortalization()
     def test_remove_subclass(self):
         # bpo-46417: when the last subclass of a type is deleted,
         # remove_subclass() clears the internal dictionary of subclasses:
diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py
@@ -1992,7 +1992,6 @@ def f():
             return 1
         self.assertEqual(f.cache_parameters(), {'maxsize': 1000, "typed": True})
 
-    @support.suppress_immortalization()
     def test_lru_cache_weakrefable(self):
         @self.module.lru_cache
         def test_function(x):
diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py
@@ -3,7 +3,7 @@
 from test import support
 from test.support import (verbose, refcount_test,
                           cpython_only, requires_subprocess,
-                          requires_gil_enabled, suppress_immortalization,
+                          requires_gil_enabled,
                           Py_GIL_DISABLED)
 from test.support.import_helper import import_module
 from test.support.os_helper import temp_dir, TESTFN, unlink
@@ -110,7 +110,6 @@ def test_tuple(self):
         del l
         self.assertEqual(gc.collect(), 2)
 
-    @suppress_immortalization()
     def test_class(self):
         class A:
             pass
@@ -119,7 +118,6 @@ class A:
         del A
         self.assertNotEqual(gc.collect(), 0)
 
-    @suppress_immortalization()
     def test_newstyleclass(self):
         class A(object):
             pass
@@ -136,7 +134,6 @@ class A:
         del a
         self.assertNotEqual(gc.collect(), 0)
 
-    @suppress_immortalization()
     def test_newinstance(self):
         class A(object):
             pass
@@ -223,7 +220,6 @@ class B(object):
             self.fail("didn't find obj in garbage (finalizer)")
         gc.garbage.remove(obj)
 
-    @suppress_immortalization()
     def test_function(self):
         # Tricky: f -> d -> f, code should call d.clear() after the exec to
         # break the cycle.
@@ -566,7 +562,6 @@ def test_get_referents(self):
 
         self.assertEqual(gc.get_referents(1, 'a', 4j), [])
 
-    @suppress_immortalization()
     def test_is_tracked(self):
         # Atomic built-in types are not tracked, user-defined objects and
         # mutable containers are.
@@ -604,9 +599,7 @@ class UserFloatSlots(float):
         class UserIntSlots(int):
             __slots__ = ()
 
-        if not Py_GIL_DISABLED:
-            # gh-117783: modules may be immortalized in free-threaded build
-            self.assertTrue(gc.is_tracked(gc))
+        self.assertTrue(gc.is_tracked(gc))
         self.assertTrue(gc.is_tracked(UserClass))
         self.assertTrue(gc.is_tracked(UserClass()))
         self.assertTrue(gc.is_tracked(UserInt()))
diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py
@@ -35,7 +35,7 @@
 except ImportError:
     ThreadPoolExecutor = None
 
-from test.support import cpython_only, import_helper, suppress_immortalization
+from test.support import cpython_only, import_helper
 from test.support import MISSING_C_DOCSTRINGS, ALWAYS_EQ
 from test.support.import_helper import DirsOnSysPath, ready_to_import
 from test.support.os_helper import TESTFN, temp_cwd
@@ -771,7 +771,6 @@ def test_getfile_builtin_function_or_method(self):
             inspect.getfile(list.append)
         self.assertIn('expected, got', str(e_append.exception))
 
-    @suppress_immortalization()
     def test_getfile_class_without_module(self):
         class CM(type):
             @property
@@ -2576,7 +2575,6 @@ def __getattribute__(self, attr):
 
         self.assertFalse(test.called)
 
-    @suppress_immortalization()
     def test_cache_does_not_cause_classes_to_persist(self):
         # regression test for gh-118013:
         # check that the internal _shadowed_dict cache does not cause
diff --git a/Lib/test/test_module/__init__.py b/Lib/test/test_module/__init__.py
@@ -4,7 +4,6 @@
 import weakref
 from test.support import gc_collect
 from test.support import import_helper
-from test.support import suppress_immortalization
 from test.support.script_helper import assert_python_ok
 
 import sys
@@ -104,7 +103,6 @@ def f():
         gc_collect()
         self.assertEqual(f().__dict__["bar"], 4)
 
-    @suppress_immortalization()
     def test_clear_dict_in_ref_cycle(self):
         destroyed = []
         m = ModuleType("foo")
@@ -120,7 +118,6 @@ def __del__(self):
         gc_collect()
         self.assertEqual(destroyed, [1])
 
-    @suppress_immortalization()
     def test_weakref(self):
         m = ModuleType("foo")
         wr = weakref.ref(m)
diff --git a/Lib/test/test_ordered_dict.py b/Lib/test/test_ordered_dict.py
@@ -12,7 +12,7 @@
 import weakref
 from collections.abc import MutableMapping
 from test import mapping_tests, support
-from test.support import import_helper, suppress_immortalization
+from test.support import import_helper
 
 
 py_coll = import_helper.import_fresh_module('collections',
@@ -669,7 +669,6 @@ def test_dict_update(self):
         dict.update(od, [('spam', 1)])
         self.assertNotIn('NULL', repr(od))
 
-    @suppress_immortalization()
     def test_reference_loop(self):
         # Issue 25935
         OrderedDict = self.OrderedDict
diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py
@@ -10,7 +10,7 @@
 import weakref
 
 from test import support
-from test.support import import_helper, suppress_immortalization
+from test.support import import_helper
 from test.support.script_helper import assert_python_ok
 from test.support.testcase import ComplexesAreIdenticalMixin
 
@@ -697,7 +697,6 @@ def __del__(self):
         self.assertIn(b"Exception ignored in:", stderr)
         self.assertIn(b"C.__del__", stderr)
 
-    @suppress_immortalization()
     def test__struct_reference_cycle_cleaned_up(self):
         # Regression test for python/cpython#94207.
 
diff --git a/Lib/test/test_trace.py b/Lib/test/test_trace.py
@@ -1,7 +1,7 @@
 import os
 from pickle import dump
 import sys
-from test.support import captured_stdout, requires_resource, requires_gil_enabled
+from test.support import captured_stdout, requires_resource
 from test.support.os_helper import (TESTFN, rmtree, unlink)
 from test.support.script_helper import assert_python_ok, assert_python_failure
 import textwrap
@@ -301,7 +301,6 @@ def test_loop_caller_importing(self):
 
     @unittest.skipIf(hasattr(sys, 'gettrace') and sys.gettrace(),
                      'pre-existing trace function throws off measurements')
-    @requires_gil_enabled("gh-117783: immortalization of types affects traced method names")
     def test_inst_method_calling(self):
         obj = TracedClass(20)
         self.tracer.runfunc(obj.inst_method_calling, 1)
@@ -335,7 +334,6 @@ def setUp(self):
 
     @unittest.skipIf(hasattr(sys, 'gettrace') and sys.gettrace(),
                      'pre-existing trace function throws off measurements')
-    @requires_gil_enabled("gh-117783: immortalization of types affects traced method names")
     def test_loop_caller_importing(self):
         self.tracer.runfunc(traced_func_importing_caller, 1)
 
diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py
diff --git a/Lib/test/test_zoneinfo/test_zoneinfo.py b/Lib/test/test_zoneinfo/test_zoneinfo.py
diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c
diff --git a/Objects/codeobject.c b/Objects/codeobject.c
diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c
