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

* 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.
This commit is contained in:
Sam Gross 2024-10-24 18:09:59 -04:00 committed by GitHub
parent 1306f33c84
commit 332356b880
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
23 changed files with 36 additions and 137 deletions

View File

@ -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
};

View File

@ -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)

View File

@ -7,8 +7,7 @@ import sysconfig
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 @@ class Regrtest:
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)

View File

@ -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()

View File

@ -426,7 +426,6 @@ class CommonTest(unittest.TestCase):
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)

View File

@ -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')

View File

@ -2259,7 +2259,7 @@ class ConstantTests(unittest.TestCase):
"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)

View File

@ -25,7 +25,6 @@ from test.support import import_helper
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 @@ class CAPITest(unittest.TestCase):
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 @@ class CAPITest(unittest.TestCase):
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):

View File

@ -4,7 +4,7 @@ import contextvars
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 @@ class TestCodeObjectWatchers(unittest.TestCase):
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 @@ class TestCodeObjectWatchers(unittest.TestCase):
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):

View File

@ -141,9 +141,7 @@ except ImportError:
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 @@ class CodeConstsTest(unittest.TestCase):
@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 @@ class CodeConstsTest(unittest.TestCase):
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 @@ if check_impl_detail(cpython=True) and ctypes is not None:
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 @@ if check_impl_detail(cpython=True) and ctypes is not None:
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.

View File

@ -5076,7 +5076,6 @@ class ClassPropertiesAndMethods(unittest.TestCase):
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:

View File

@ -1992,7 +1992,6 @@ class TestLRU:
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):

View File

@ -3,7 +3,7 @@ import unittest.mock
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 @@ class GCTests(unittest.TestCase):
del l
self.assertEqual(gc.collect(), 2)
@suppress_immortalization()
def test_class(self):
class A:
pass
@ -119,7 +118,6 @@ class GCTests(unittest.TestCase):
del A
self.assertNotEqual(gc.collect(), 0)
@suppress_immortalization()
def test_newstyleclass(self):
class A(object):
pass
@ -136,7 +134,6 @@ class GCTests(unittest.TestCase):
del a
self.assertNotEqual(gc.collect(), 0)
@suppress_immortalization()
def test_newinstance(self):
class A(object):
pass
@ -223,7 +220,6 @@ class GCTests(unittest.TestCase):
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 @@ class GCTests(unittest.TestCase):
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 GCTests(unittest.TestCase):
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()))

View File

@ -35,7 +35,7 @@ try:
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 @@ class TestRetrievingSourceCode(GetSourceBase):
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 @@ class TestGetattrStatic(unittest.TestCase):
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

View File

@ -4,7 +4,6 @@ import unittest
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 @@ class ModuleTests(unittest.TestCase):
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 @@ a = A(destroyed)"""
gc_collect()
self.assertEqual(destroyed, [1])
@suppress_immortalization()
def test_weakref(self):
m = ModuleType("foo")
wr = weakref.ref(m)

View File

@ -12,7 +12,7 @@ import unittest
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 @@ class OrderedDictTests:
dict.update(od, [('spam', 1)])
self.assertNotIn('NULL', repr(od))
@suppress_immortalization()
def test_reference_loop(self):
# Issue 25935
OrderedDict = self.OrderedDict

View File

@ -10,7 +10,7 @@ import sys
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 @@ class StructTest(ComplexesAreIdenticalMixin, unittest.TestCase):
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.

View File

@ -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 @@ class TestFuncs(unittest.TestCase):
@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 @@ class TestCallers(unittest.TestCase):
@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)

View File

@ -13,7 +13,7 @@ import random
import textwrap
from test import support
from test.support import script_helper, ALWAYS_EQ, suppress_immortalization
from test.support import script_helper, ALWAYS_EQ
from test.support import gc_collect
from test.support import import_helper
from test.support import threading_helper
@ -659,7 +659,6 @@ class ReferencesTestCase(TestBase):
# deallocation of c2.
del c2
@suppress_immortalization()
def test_callback_in_cycle(self):
import gc
@ -752,7 +751,6 @@ class ReferencesTestCase(TestBase):
del c1, c2, C, D
gc.collect()
@suppress_immortalization()
def test_callback_in_cycle_resurrection(self):
import gc
@ -888,7 +886,6 @@ class ReferencesTestCase(TestBase):
# No exception should be raised here
gc.collect()
@suppress_immortalization()
def test_classes(self):
# Check that classes are weakrefable.
class A(object):

View File

@ -17,7 +17,7 @@ import unittest
from datetime import date, datetime, time, timedelta, timezone
from functools import cached_property
from test.support import MISSING_C_DOCSTRINGS, requires_gil_enabled
from test.support import MISSING_C_DOCSTRINGS
from test.test_zoneinfo import _support as test_support
from test.test_zoneinfo._support import OS_ENV_LOCK, TZPATH_TEST_LOCK, ZoneInfoTestBase
from test.support.import_helper import import_module, CleanImport
@ -1931,7 +1931,6 @@ class ExtensionBuiltTest(unittest.TestCase):
self.assertFalse(hasattr(c_zoneinfo.ZoneInfo, "_weak_cache"))
self.assertTrue(hasattr(py_zoneinfo.ZoneInfo, "_weak_cache"))
@requires_gil_enabled("gh-117783: types may be immortalized")
def test_gc_tracked(self):
import gc

View File

@ -1965,32 +1965,6 @@ get_py_thread_id(PyObject *self, PyObject *Py_UNUSED(ignored))
}
#endif
static PyObject *
suppress_immortalization(PyObject *self, PyObject *value)
{
#ifdef Py_GIL_DISABLED
int suppress = PyObject_IsTrue(value);
if (suppress < 0) {
return NULL;
}
PyInterpreterState *interp = PyInterpreterState_Get();
// Subtract two to suppress immortalization (so that 1 -> -1)
_Py_atomic_add_int(&interp->gc.immortalize, suppress ? -2 : 2);
#endif
Py_RETURN_NONE;
}
static PyObject *
get_immortalize_deferred(PyObject *self, PyObject *Py_UNUSED(ignored))
{
#ifdef Py_GIL_DISABLED
PyInterpreterState *interp = PyInterpreterState_Get();
return PyBool_FromLong(_Py_atomic_load_int(&interp->gc.immortalize) >= 0);
#else
Py_RETURN_FALSE;
#endif
}
static PyObject *
has_inline_values(PyObject *self, PyObject *obj)
{
@ -2137,8 +2111,6 @@ static PyMethodDef module_functions[] = {
#ifdef Py_GIL_DISABLED
{"py_thread_id", get_py_thread_id, METH_NOARGS},
#endif
{"suppress_immortalization", suppress_immortalization, METH_O},
{"get_immortalize_deferred", get_immortalize_deferred, METH_NOARGS},
#ifdef _Py_TIER2
{"uop_symbols_test", _Py_uop_symbols_test, METH_NOARGS},
#endif

View File

@ -108,14 +108,8 @@ should_intern_string(PyObject *o)
{
#ifdef Py_GIL_DISABLED
// The free-threaded build interns (and immortalizes) all string constants
// unless we've disabled immortalizing objects that use deferred reference
// counting.
PyInterpreterState *interp = _PyInterpreterState_GET();
if (_Py_atomic_load_int(&interp->gc.immortalize) < 0) {
return 1;
}
#endif
return 1;
#else
// compute if s matches [a-zA-Z0-9_]
const unsigned char *s, *e;
@ -129,6 +123,7 @@ should_intern_string(PyObject *o)
return 0;
}
return 1;
#endif
}
#ifdef Py_GIL_DISABLED
@ -237,13 +232,10 @@ intern_constants(PyObject *tuple, int *modified)
Py_DECREF(tmp);
}
// Intern non-string constants in the free-threaded build, but only if
// we are also immortalizing objects that use deferred reference
// counting.
PyThreadState *tstate = PyThreadState_GET();
// Intern non-string constants in the free-threaded build
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET();
if (!_Py_IsImmortal(v) && !PyCode_Check(v) &&
!PyUnicode_CheckExact(v) &&
_Py_atomic_load_int(&tstate->interp->gc.immortalize) >= 0)
!PyUnicode_CheckExact(v) && !tstate->suppress_co_const_immortalization)
{
PyObject *interned = intern_one_constant(v);
if (interned == NULL) {

View File

@ -867,18 +867,17 @@ builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename,
goto error;
#ifdef Py_GIL_DISABLED
// gh-118527: Disable immortalization of code constants for explicit
// Disable immortalization of code constants for explicit
// compile() calls to get consistent frozen outputs between the default
// and free-threaded builds.
// Subtract two to suppress immortalization (so that 1 -> -1)
PyInterpreterState *interp = _PyInterpreterState_GET();
_Py_atomic_add_int(&interp->gc.immortalize, -2);
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET();
tstate->suppress_co_const_immortalization++;
#endif
result = Py_CompileStringObject(str, filename, start[compile_mode], &cf, optimize);
#ifdef Py_GIL_DISABLED
_Py_atomic_add_int(&interp->gc.immortalize, 2);
tstate->suppress_co_const_immortalization--;
#endif
Py_XDECREF(source_copy);
@ -1024,7 +1023,16 @@ builtin_eval_impl(PyObject *module, PyObject *source, PyObject *globals,
str++;
(void)PyEval_MergeCompilerFlags(&cf);
#ifdef Py_GIL_DISABLED
// Don't immortalize code constants for explicit eval() calls
// to avoid memory leaks.
_PyThreadStateImpl *tstate = (_PyThreadStateImpl *)_PyThreadState_GET();
tstate->suppress_co_const_immortalization++;
#endif
result = PyRun_StringFlags(str, Py_eval_input, globals, locals, &cf);
#ifdef Py_GIL_DISABLED
tstate->suppress_co_const_immortalization--;
#endif
Py_XDECREF(source_copy);
}