From db000e81311d1060ed7b5eb5ad3d702da538999c Mon Sep 17 00:00:00 2001 From: sunmy2019 <59365878+sunmy2019@users.noreply.github.com> Date: Tue, 23 May 2023 18:26:16 +0800 Subject: [PATCH 1/7] add some cleanups --- Lib/test/libregrtest/main.py | 7 +++++++ Lib/test/test_import/__init__.py | 9 ++++++++- Modules/_testsinglephase.c | 26 +++++++++++++++++++++----- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py index 3c3509d0303371..8fb7be5c857492 100644 --- a/Lib/test/libregrtest/main.py +++ b/Lib/test/libregrtest/main.py @@ -213,6 +213,13 @@ def parse_args(self, kwargs): if ns.tempdir: ns.tempdir = os.path.expanduser(ns.tempdir) + # introduce a way to skip the ref leak tests + if ns.huntrleaks: + if ns.ignore_tests is None: + ns.ignore_tests = ["*skip_ref_leak_test"] + else: + ns.ignore_tests.append("*skip_ref_leak_test") + self.ns = ns def find_tests(self, tests): diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index e2384a08ecaa90..363dc1b3df5b09 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -2330,6 +2330,8 @@ def test_variants(self): self.assertIs(basic.look_up_self(), basic_lookedup) self.assertEqual(basic.initialized_count(), expected_init_count) + loaded.module._clear_module_state() + def test_basic_reloaded(self): # m_copy is copied into the existing module object. # Global state is not changed. @@ -2413,6 +2415,11 @@ def test_with_reinit_reloaded(self): self.assertIs(reloaded.snapshot.cached, reloaded.module) + if hasattr(loaded.module, '_clear_module_state'): + loaded.module._clear_module_state() + if hasattr(reloaded.module, '_clear_module_state'): + reloaded.module._clear_module_state() + # Currently, for every single-phrase init module loaded # in multiple interpreters, those interpreters share a # PyModuleDef for that object, which can be a problem. @@ -2488,7 +2495,7 @@ def test_basic_multiple_interpreters_main_no_reset(self): # * module's global state was updated, not reset @requires_subinterpreters - def test_basic_multiple_interpreters_deleted_no_reset(self): + def test_basic_multiple_interpreters_deleted_no_reset_skip_ref_leak_test(self): # without resetting; already loaded in a deleted interpreter # At this point: diff --git a/Modules/_testsinglephase.c b/Modules/_testsinglephase.c index a16157702ae789..6758b432d29bb8 100644 --- a/Modules/_testsinglephase.c +++ b/Modules/_testsinglephase.c @@ -137,13 +137,10 @@ init_module(PyObject *module, module_state *state) double d = _PyTime_AsSecondsDouble(state->initialized); PyObject *initialized = PyFloat_FromDouble(d); - if (initialized == NULL) { - return -1; - } if (PyModule_AddObjectRef(module, "_module_initialized", initialized) != 0) { return -1; } - + Py_XDECREF(initialized); return 0; } @@ -246,6 +243,24 @@ basic__clear_globals(PyObject *self, PyObject *Py_UNUSED(ignored)) basic__clear_globals_doc} +PyDoc_STRVAR(basic__clear_module_state_doc, "_clear_module_state()\n\ +\n\ +Free the module state and set it to uninitialized."); + +static PyObject* +basic__clear_module_state(PyObject *self, PyObject *Py_UNUSED(ignored)) { + module_state *state = get_module_state(self); + if (state != NULL) { + clear_state(state); + } + Py_RETURN_NONE; +} + +#define _CLEAR_MODULE_STATE_METHODDEF \ + {"_clear_module_state", basic__clear_module_state, METH_NOARGS, \ + basic__clear_module_state_doc} + + /*********************************************/ /* the _testsinglephase module (and aliases) */ /*********************************************/ @@ -406,7 +421,7 @@ PyInit__testsinglephase_with_reinit(void) /* the _testsinglephase_with_state module */ /******************************************/ -/* This ia less typical of legacy extensions in the wild: +/* This is a less typical of legacy extensions in the wild: - single-phase init (same as _testsinglephase above) - has some module state - supports repeated initialization @@ -422,6 +437,7 @@ static PyMethodDef TestMethods_WithState[] = { LOOK_UP_SELF_METHODDEF, SUM_METHODDEF, STATE_INITIALIZED_METHODDEF, + _CLEAR_MODULE_STATE_METHODDEF, {NULL, NULL} /* sentinel */ }; From e6bc99c18eff7466378cf045486fa8a79266534b Mon Sep 17 00:00:00 2001 From: sunmy2019 <59365878+sunmy2019@users.noreply.github.com> Date: Tue, 23 May 2023 22:03:12 +0800 Subject: [PATCH 2/7] remove the sys.argv trick --- Lib/test/test_import/__init__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 363dc1b3df5b09..ca21082e0c03b2 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -1980,10 +1980,6 @@ class SinglephaseInitTests(unittest.TestCase): @classmethod def setUpClass(cls): - if '-R' in sys.argv or '--huntrleaks' in sys.argv: - # https://github.com/python/cpython/issues/102251 - raise unittest.SkipTest('unresolved refleaks (see gh-102251)') - spec = importlib.util.find_spec(cls.NAME) from importlib.machinery import ExtensionFileLoader cls.FILE = spec.origin From c6b42ee888e28cdfb4720adc1405b331c7f3fa99 Mon Sep 17 00:00:00 2001 From: sunmy2019 <59365878+sunmy2019@users.noreply.github.com> Date: Tue, 30 May 2023 11:37:35 +0800 Subject: [PATCH 3/7] follow @erlend-aasland's advice --- Modules/_testsinglephase.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Modules/_testsinglephase.c b/Modules/_testsinglephase.c index 6758b432d29bb8..49e60db429dc88 100644 --- a/Modules/_testsinglephase.c +++ b/Modules/_testsinglephase.c @@ -137,10 +137,14 @@ init_module(PyObject *module, module_state *state) double d = _PyTime_AsSecondsDouble(state->initialized); PyObject *initialized = PyFloat_FromDouble(d); - if (PyModule_AddObjectRef(module, "_module_initialized", initialized) != 0) { + if (initialized == NULL) { + return -1; + } + int rc = PyModule_AddObjectRef(module, "_module_initialized", initialized); + Py_DECREF(initialized); + if (rc != 0) { return -1; } - Py_XDECREF(initialized); return 0; } From 2ee60e227ff46d7ef2d90d5837eb05d5ce4a8b32 Mon Sep 17 00:00:00 2001 From: sunmy2019 <59365878+sunmy2019@users.noreply.github.com> Date: Tue, 30 May 2023 17:11:38 +0800 Subject: [PATCH 4/7] revert this as this would be intended at gh-104796 --- Modules/_testsinglephase.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Modules/_testsinglephase.c b/Modules/_testsinglephase.c index 49e60db429dc88..79d1672c9de9a3 100644 --- a/Modules/_testsinglephase.c +++ b/Modules/_testsinglephase.c @@ -140,11 +140,10 @@ init_module(PyObject *module, module_state *state) if (initialized == NULL) { return -1; } - int rc = PyModule_AddObjectRef(module, "_module_initialized", initialized); - Py_DECREF(initialized); - if (rc != 0) { + if (PyModule_AddObjectRef(module, "_module_initialized", initialized) != 0) { return -1; } + return 0; } From 695fd16ae82064c57dcc57298d109e3a6612b575 Mon Sep 17 00:00:00 2001 From: sunmy2019 <59365878+sunmy2019@users.noreply.github.com> Date: Thu, 1 Jun 2023 10:29:44 +0800 Subject: [PATCH 5/7] rebase after changes to #105085 --- Lib/test/test_import/__init__.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index 7e9208578be07b..7e8db687939028 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -2327,8 +2327,6 @@ def test_variants(self): self.assertIs(basic.look_up_self(), basic_lookedup) self.assertEqual(basic.initialized_count(), expected_init_count) - loaded.module._clear_module_state() - def test_basic_reloaded(self): # m_copy is copied into the existing module object. # Global state is not changed. @@ -2417,11 +2415,6 @@ def test_with_reinit_reloaded(self): self.assertIs(reloaded.snapshot.cached, reloaded.module) - if hasattr(loaded.module, '_clear_module_state'): - loaded.module._clear_module_state() - if hasattr(reloaded.module, '_clear_module_state'): - reloaded.module._clear_module_state() - # Currently, for every single-phrase init module loaded # in multiple interpreters, those interpreters share a # PyModuleDef for that object, which can be a problem. From 5934e502882376dc694d7502714513f11946feab Mon Sep 17 00:00:00 2001 From: sunmy2019 <59365878+sunmy2019@users.noreply.github.com> Date: Fri, 23 Jun 2023 12:25:08 +0800 Subject: [PATCH 6/7] collect tests in a single class --- Lib/test/libregrtest/main.py | 7 -- Lib/test/test_import/__init__.py | 120 +++++++++++++++++-------------- 2 files changed, 67 insertions(+), 60 deletions(-) diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py index 4392a2058ed47a..9001ca33b6166a 100644 --- a/Lib/test/libregrtest/main.py +++ b/Lib/test/libregrtest/main.py @@ -213,13 +213,6 @@ def parse_args(self, kwargs): if ns.tempdir: ns.tempdir = os.path.expanduser(ns.tempdir) - # introduce a way to skip the ref leak tests - if ns.huntrleaks: - if ns.ignore_tests is None: - ns.ignore_tests = ["*skip_ref_leak_test"] - else: - ns.ignore_tests.append("*skip_ref_leak_test") - self.ns = ns def find_tests(self, tests): diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index bacb5324f38132..b4836335b171f3 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -1982,9 +1982,7 @@ def parse(cls, text): return self -@requires_singlephase_init -class SinglephaseInitTests(unittest.TestCase): - +class SinglephaseInitTestMixin: NAME = '_testsinglephase' @classmethod @@ -2236,6 +2234,8 @@ def check_copied(self, loaded, base): self.assertEqual(snap.state_initialized, base.snapshot.state_initialized) +@requires_singlephase_init +class SinglephaseInitTests(SinglephaseInitTestMixin, unittest.TestCase): ######################### # the tests @@ -2499,8 +2499,8 @@ def test_basic_multiple_interpreters_main_no_reset(self): # * module's global state was updated, not reset @requires_subinterpreters - def test_basic_multiple_interpreters_deleted_no_reset_skip_ref_leak_test(self): - # without resetting; already loaded in a deleted interpreter + def test_basic_multiple_interpreters_reset_each(self): + # resetting between each interpreter # At this point: # * alive in 0 interpreters @@ -2514,61 +2514,65 @@ def test_basic_multiple_interpreters_deleted_no_reset_skip_ref_leak_test(self): interpid1 = self.add_subinterpreter() interpid2 = self.add_subinterpreter() - # First, load in the main interpreter but then completely clear it. - loaded_main = self.load(self.NAME) - loaded_main.module._clear_globals() - _testinternalcapi.clear_extension(self.NAME, self.FILE) - - # At this point: - # * alive in 0 interpreters - # * module def loaded already - # * module def was in _PyRuntime.imports.extensions, but cleared - # * mod init func ran for the first time (since reset, at least) - # * m_copy was set, but cleared (was NULL) - # * module's global state was initialized but cleared - - # Start with an interpreter that gets destroyed right away. - base = self.import_in_subinterp(postscript=''' + # Use an interpreter that gets destroyed right away. + loaded = self.import_in_subinterp( + postscript=''' # Attrs set after loading are not in m_copy. mod.spam = 'spam, spam, mash, spam, eggs, and spam' - ''') - self.check_common(base) - self.check_fresh(base) + ''', + postcleanup=True, + ) + self.check_common(loaded) + self.check_fresh(loaded) # At this point: # * alive in 0 interpreters # * module def in _PyRuntime.imports.extensions - # * mod init func ran again + # * mod init func ran for the first time (since reset, at least) # * m_copy is NULL (claered when the interpreter was destroyed) # * module's global state was initialized, not reset # Use a subinterpreter that sticks around. - loaded_interp1 = self.import_in_subinterp(interpid1) - self.check_common(loaded_interp1) - self.check_semi_fresh(loaded_interp1, loaded_main, base) + loaded = self.import_in_subinterp(interpid1, postcleanup=True) + self.check_common(loaded) + self.check_fresh(loaded) # At this point: # * alive in 1 interpreter (interp1) # * module def still in _PyRuntime.imports.extensions # * mod init func ran again # * m_copy was copied from interp1 (was NULL) - # * module's global state was updated, not reset + # * module's global state was initialized, not reset # Use a subinterpreter while the previous one is still alive. - loaded_interp2 = self.import_in_subinterp(interpid2) - self.check_common(loaded_interp2) - self.check_copied(loaded_interp2, loaded_interp1) + loaded = self.import_in_subinterp(interpid2, postcleanup=True) + self.check_common(loaded) + self.check_fresh(loaded) # At this point: - # * alive in 2 interpreters (interp1, interp2) + # * alive in 2 interpreters (interp2, interp2) # * module def still in _PyRuntime.imports.extensions # * mod init func ran again # * m_copy was copied from interp2 (was from interp1) - # * module's global state was updated, not reset + # * module's global state was initialized, not reset + + +@requires_singlephase_init +class SinglephaseInitTestsNoRerun(SinglephaseInitTestMixin, unittest.TestCase): + # Tests does not support rerunning are collected in this class + _has_run = False + + @classmethod + def setUpClass(cls): + super().setUpClass() + if cls._has_run: + raise unittest.SkipTest("requires single-phase init") + cls._has_run = True @requires_subinterpreters - def test_basic_multiple_interpreters_reset_each(self): - # resetting between each interpreter + def test_basic_multiple_interpreters_deleted_no_reset(self): + # without resetting; already loaded in a deleted interpreter + # This test intentionally leaks references # At this point: # * alive in 0 interpreters @@ -2582,47 +2586,57 @@ def test_basic_multiple_interpreters_reset_each(self): interpid1 = self.add_subinterpreter() interpid2 = self.add_subinterpreter() - # Use an interpreter that gets destroyed right away. - loaded = self.import_in_subinterp( - postscript=''' + # First, load in the main interpreter but then completely clear it. + loaded_main = self.load(self.NAME) + loaded_main.module._clear_globals() + _testinternalcapi.clear_extension(self.NAME, self.FILE) + + # At this point: + # * alive in 0 interpreters + # * module def loaded already + # * module def was in _PyRuntime.imports.extensions, but cleared + # * mod init func ran for the first time (since reset, at least) + # * m_copy was set, but cleared (was NULL) + # * module's global state was initialized but cleared + + # Start with an interpreter that gets destroyed right away. + base = self.import_in_subinterp(postscript=''' # Attrs set after loading are not in m_copy. mod.spam = 'spam, spam, mash, spam, eggs, and spam' - ''', - postcleanup=True, - ) - self.check_common(loaded) - self.check_fresh(loaded) + ''') + self.check_common(base) + self.check_fresh(base) # At this point: # * alive in 0 interpreters # * module def in _PyRuntime.imports.extensions - # * mod init func ran for the first time (since reset, at least) + # * mod init func ran again # * m_copy is NULL (claered when the interpreter was destroyed) # * module's global state was initialized, not reset # Use a subinterpreter that sticks around. - loaded = self.import_in_subinterp(interpid1, postcleanup=True) - self.check_common(loaded) - self.check_fresh(loaded) + loaded_interp1 = self.import_in_subinterp(interpid1) + self.check_common(loaded_interp1) + self.check_semi_fresh(loaded_interp1, loaded_main, base) # At this point: # * alive in 1 interpreter (interp1) # * module def still in _PyRuntime.imports.extensions # * mod init func ran again # * m_copy was copied from interp1 (was NULL) - # * module's global state was initialized, not reset + # * module's global state was updated, not reset # Use a subinterpreter while the previous one is still alive. - loaded = self.import_in_subinterp(interpid2, postcleanup=True) - self.check_common(loaded) - self.check_fresh(loaded) + loaded_interp2 = self.import_in_subinterp(interpid2) + self.check_common(loaded_interp2) + self.check_copied(loaded_interp2, loaded_interp1) # At this point: - # * alive in 2 interpreters (interp2, interp2) + # * alive in 2 interpreters (interp1, interp2) # * module def still in _PyRuntime.imports.extensions # * mod init func ran again # * m_copy was copied from interp2 (was from interp1) - # * module's global state was initialized, not reset + # * module's global state was updated, not reset @cpython_only From 60b4921865d178c6f5d7f1075a9ec77fa049602c Mon Sep 17 00:00:00 2001 From: sunmy2019 <59365878+sunmy2019@users.noreply.github.com> Date: Fri, 23 Jun 2023 12:31:07 +0800 Subject: [PATCH 7/7] Update Lib/test/test_import/__init__.py --- Lib/test/test_import/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_import/__init__.py b/Lib/test/test_import/__init__.py index b4836335b171f3..44848da036acfd 100644 --- a/Lib/test/test_import/__init__.py +++ b/Lib/test/test_import/__init__.py @@ -2566,7 +2566,7 @@ class SinglephaseInitTestsNoRerun(SinglephaseInitTestMixin, unittest.TestCase): def setUpClass(cls): super().setUpClass() if cls._has_run: - raise unittest.SkipTest("requires single-phase init") + raise unittest.SkipTest("Tests do not support rerunning") cls._has_run = True @requires_subinterpreters