From ff101340979ca6f668a12e8ad8754fe44b78a626 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 1 Jul 2025 06:12:10 +0900 Subject: [PATCH 01/11] NEWS --- .../next/Library/2025-06-30-15-12-33.gh-issue-132413.TvpIn2.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2025-06-30-15-12-33.gh-issue-132413.TvpIn2.rst diff --git a/Misc/NEWS.d/next/Library/2025-06-30-15-12-33.gh-issue-132413.TvpIn2.rst b/Misc/NEWS.d/next/Library/2025-06-30-15-12-33.gh-issue-132413.TvpIn2.rst new file mode 100644 index 00000000000000..3e778bf54f8e02 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-06-30-15-12-33.gh-issue-132413.TvpIn2.rst @@ -0,0 +1 @@ +Fix crash in C version of :mod:`datetime` when used during interpreter shutdown. From 4f83cb00dd83b10090ed0611db2d15a0486ba40d Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 1 Jul 2025 06:15:02 +0900 Subject: [PATCH 02/11] Add test --- Lib/test/datetimetester.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 93b3382b9c654e..668e1b42d6eaa0 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7295,6 +7295,25 @@ def test_update_type_cache(self): """) script_helper.assert_python_ok('-c', script) + def test_static_type_at_shutdown(self): + # gh-132413 + script = textwrap.dedent(""" + import _datetime + timedelta = _datetime.timedelta + + def gen(): + try: + yield + finally: + # sys.modules is empty + _datetime.timedelta(days=1) + timedelta(days=1) + + it = gen() + next(it) + """) + script_helper.assert_python_ok('-c', script) + def load_tests(loader, standard_tests, pattern): standard_tests.addTest(ZoneInfoCompleteTest()) From 4e83b5b3e302b635120d47d09f8119ce6f8b9a2b Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 1 Jul 2025 06:31:51 +0900 Subject: [PATCH 03/11] Avoid weakref being cleared by GC --- Python/pylifecycle.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 724fda63511282..f9f2601356bd58 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1554,7 +1554,17 @@ finalize_remove_modules(PyObject *modules, int verbose) if (weaklist != NULL) { \ PyObject *wr = PyWeakref_NewRef(mod, NULL); \ if (wr) { \ - PyObject *tup = PyTuple_Pack(2, name, wr); \ + PyObject *tup; \ + if (Py_REFCNT(wr) > 1) { \ + /* gh-132413: When the weakref is already used + * elsewhere, keep the referenced module alive + * until finalize_modules_clear_weaklist() finishes. + */ \ + tup = PyTuple_Pack(3, name, wr, mod); \ + } \ + else { \ + tup = PyTuple_Pack(2, name, wr); \ + } \ if (!tup || PyList_Append(weaklist, tup) < 0) { \ PyErr_FormatUnraisable("Exception ignored while removing modules"); \ } \ From c4b1c62ea0dadc164097943c86c6e46181901335 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 1 Jul 2025 09:23:25 +0900 Subject: [PATCH 04/11] Do not hold module in tuple --- Python/pylifecycle.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index f9f2601356bd58..a32e2a46c8439b 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1560,10 +1560,11 @@ finalize_remove_modules(PyObject *modules, int verbose) * elsewhere, keep the referenced module alive * until finalize_modules_clear_weaklist() finishes. */ \ - tup = PyTuple_Pack(3, name, wr, mod); \ + Py_INCREF(mod); \ + tup = PyTuple_Pack(3, name, wr, Py_True); \ } \ else { \ - tup = PyTuple_Pack(2, name, wr); \ + tup = PyTuple_Pack(3, name, wr, Py_False); \ } \ if (!tup || PyList_Append(weaklist, tup) < 0) { \ PyErr_FormatUnraisable("Exception ignored while removing modules"); \ @@ -1677,6 +1678,9 @@ finalize_modules_clear_weaklist(PyInterpreterState *interp, } _PyModule_Clear(mod); Py_DECREF(mod); + if (PyTuple_GET_ITEM(tup, 2) == Py_True) { + Py_DECREF(mod); + } } } From 4b1b45c44da356fa36585814e4b58d8d44c8a75b Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 1 Jul 2025 17:46:08 +0900 Subject: [PATCH 05/11] GC-untrack/track --- Python/pylifecycle.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index a32e2a46c8439b..7c396217f5efe0 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1554,18 +1554,14 @@ finalize_remove_modules(PyObject *modules, int verbose) if (weaklist != NULL) { \ PyObject *wr = PyWeakref_NewRef(mod, NULL); \ if (wr) { \ - PyObject *tup; \ if (Py_REFCNT(wr) > 1) { \ - /* gh-132413: When the weakref is already used - * elsewhere, keep the referenced module alive - * until finalize_modules_clear_weaklist() finishes. - */ \ - Py_INCREF(mod); \ - tup = PyTuple_Pack(3, name, wr, Py_True); \ - } \ - else { \ - tup = PyTuple_Pack(3, name, wr, Py_False); \ + /* gh-132413: When the weakref is already used elsewhere, + * finalize_modules_clear_weaklist() rather than the GC + * should clear the referenced module since the GC tries + * to clear the wrakref first. */ \ + _PyObject_GC_UNTRACK(mod); \ } \ + PyObject *tup = PyTuple_Pack(2, name, wr); \ if (!tup || PyList_Append(weaklist, tup) < 0) { \ PyErr_FormatUnraisable("Exception ignored while removing modules"); \ } \ @@ -1668,6 +1664,9 @@ finalize_modules_clear_weaklist(PyInterpreterState *interp, continue; } assert(PyModule_Check(mod)); + if (!_PyObject_GC_IS_TRACKED(mod)) { + _PyObject_GC_TRACK(mod); + } PyObject *dict = _PyModule_GetDict(mod); // borrowed reference if (dict == interp->builtins || dict == interp->sysdict) { Py_DECREF(mod); @@ -1678,9 +1677,6 @@ finalize_modules_clear_weaklist(PyInterpreterState *interp, } _PyModule_Clear(mod); Py_DECREF(mod); - if (PyTuple_GET_ITEM(tup, 2) == Py_True) { - Py_DECREF(mod); - } } } From e33f704cde0c6eddf664fd2d32ad7d2ca255e3f4 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 2 Jul 2025 02:35:41 +0900 Subject: [PATCH 06/11] Sort weaklist --- Python/pylifecycle.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 7c396217f5efe0..b44337b018a8f7 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1546,6 +1546,10 @@ static PyObject* finalize_remove_modules(PyObject *modules, int verbose) { PyObject *weaklist = PyList_New(0); + PyObject *weak_ext = PyList_New(0); // for extending the weaklist + if (weak_ext == NULL) { + Py_CLEAR(weaklist); + } if (weaklist == NULL) { PyErr_FormatUnraisable("Exception ignored while removing modules"); } @@ -1554,17 +1558,19 @@ finalize_remove_modules(PyObject *modules, int verbose) if (weaklist != NULL) { \ PyObject *wr = PyWeakref_NewRef(mod, NULL); \ if (wr) { \ - if (Py_REFCNT(wr) > 1) { \ + PyObject *list = Py_REFCNT(wr) <= 1 ? weaklist : weak_ext; \ + PyObject *tup = PyTuple_Pack(2, name, wr); \ + if (!tup || PyList_Append(list, tup) < 0) { \ + PyErr_FormatUnraisable("Exception ignored while removing modules"); \ + } \ + if (list == weak_ext) { \ /* gh-132413: When the weakref is already used elsewhere, * finalize_modules_clear_weaklist() rather than the GC * should clear the referenced module since the GC tries - * to clear the wrakref first. */ \ + * to clear the wrakref first. The weaklist requires the + * order in which such modules are cleared first. */ \ _PyObject_GC_UNTRACK(mod); \ } \ - PyObject *tup = PyTuple_Pack(2, name, wr); \ - if (!tup || PyList_Append(weaklist, tup) < 0) { \ - PyErr_FormatUnraisable("Exception ignored while removing modules"); \ - } \ Py_XDECREF(tup); \ Py_DECREF(wr); \ } \ @@ -1614,6 +1620,12 @@ finalize_remove_modules(PyObject *modules, int verbose) Py_DECREF(iterator); } } + + if (PyList_Extend(weaklist, weak_ext) < 0) { + PyErr_FormatUnraisable("Exception ignored while removing modules"); + } + Py_DECREF(weak_ext); + #undef CLEAR_MODULE #undef STORE_MODULE_WEAKREF From 263bdb2f94e32a3b1dcd006bc87839fbbbac8df5 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 2 Jul 2025 03:42:16 +0900 Subject: [PATCH 07/11] Add non-NULL check --- Python/pylifecycle.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index b44337b018a8f7..49c273d92e4e2e 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1621,10 +1621,12 @@ finalize_remove_modules(PyObject *modules, int verbose) } } - if (PyList_Extend(weaklist, weak_ext) < 0) { - PyErr_FormatUnraisable("Exception ignored while removing modules"); + if (weaklist != NULL) { + if (PyList_Extend(weaklist, weak_ext) < 0) { + PyErr_FormatUnraisable("Exception ignored while removing modules"); + } + Py_DECREF(weak_ext); } - Py_DECREF(weak_ext); #undef CLEAR_MODULE #undef STORE_MODULE_WEAKREF From fdfadf0b79d9651d9d8213bd8ad08f786809f8ba Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 2 Jul 2025 04:20:48 +0900 Subject: [PATCH 08/11] Simply do not GC-untrack/track The given module can already be untracked. --- Python/pylifecycle.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 49c273d92e4e2e..be3ce6b9cdaf39 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1558,18 +1558,23 @@ finalize_remove_modules(PyObject *modules, int verbose) if (weaklist != NULL) { \ PyObject *wr = PyWeakref_NewRef(mod, NULL); \ if (wr) { \ - PyObject *list = Py_REFCNT(wr) <= 1 ? weaklist : weak_ext; \ - PyObject *tup = PyTuple_Pack(2, name, wr); \ - if (!tup || PyList_Append(list, tup) < 0) { \ - PyErr_FormatUnraisable("Exception ignored while removing modules"); \ - } \ - if (list == weak_ext) { \ + PyObject *list; \ + PyObject *tup; \ + if (Py_REFCNT(wr) > 1) { \ /* gh-132413: When the weakref is already used elsewhere, * finalize_modules_clear_weaklist() rather than the GC * should clear the referenced module since the GC tries * to clear the wrakref first. The weaklist requires the * order in which such modules are cleared first. */ \ - _PyObject_GC_UNTRACK(mod); \ + tup = PyTuple_Pack(3, name, wr, mod); \ + list = weak_ext; \ + } \ + else { \ + tup = PyTuple_Pack(2, name, wr); \ + list = weaklist; \ + } \ + if (!tup || PyList_Append(list, tup) < 0) { \ + PyErr_FormatUnraisable("Exception ignored while removing modules"); \ } \ Py_XDECREF(tup); \ Py_DECREF(wr); \ @@ -1678,9 +1683,6 @@ finalize_modules_clear_weaklist(PyInterpreterState *interp, continue; } assert(PyModule_Check(mod)); - if (!_PyObject_GC_IS_TRACKED(mod)) { - _PyObject_GC_TRACK(mod); - } PyObject *dict = _PyModule_GetDict(mod); // borrowed reference if (dict == interp->builtins || dict == interp->sysdict) { Py_DECREF(mod); From 26c6425a431d5d19a54f90478c89ce02c11793be Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 2 Jul 2025 04:58:48 +0900 Subject: [PATCH 09/11] Add a missing NULL check --- Python/pylifecycle.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index be3ce6b9cdaf39..7fe5e96c70a1af 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1546,9 +1546,12 @@ static PyObject* finalize_remove_modules(PyObject *modules, int verbose) { PyObject *weaklist = PyList_New(0); - PyObject *weak_ext = PyList_New(0); // for extending the weaklist - if (weak_ext == NULL) { - Py_CLEAR(weaklist); + PyObject *weak_ext = NULL; // for extending the weaklist + if (weaklist != NULL) { + weak_ext = PyList_New(0); + if (weak_ext == NULL) { + Py_CLEAR(weaklist); + } } if (weaklist == NULL) { PyErr_FormatUnraisable("Exception ignored while removing modules"); From a5f2521e86fbe7a829cea0dad02b37b97aeb0eeb Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 3 Jul 2025 23:39:56 +0900 Subject: [PATCH 10/11] move test to test_gc.py --- Lib/test/datetimetester.py | 19 ------------------- Lib/test/test_gc.py | 23 +++++++++++++++++++++++ 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 668e1b42d6eaa0..93b3382b9c654e 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7295,25 +7295,6 @@ def test_update_type_cache(self): """) script_helper.assert_python_ok('-c', script) - def test_static_type_at_shutdown(self): - # gh-132413 - script = textwrap.dedent(""" - import _datetime - timedelta = _datetime.timedelta - - def gen(): - try: - yield - finally: - # sys.modules is empty - _datetime.timedelta(days=1) - timedelta(days=1) - - it = gen() - next(it) - """) - script_helper.assert_python_ok('-c', script) - def load_tests(loader, standard_tests, pattern): standard_tests.addTest(ZoneInfoCompleteTest()) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index b4cbfb6d774080..b0d0c68d6f1dc8 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1517,6 +1517,29 @@ def test_ast_fini(self): """) assert_python_ok("-c", code) + def test_weakref_to_module_at_shutdown(self): + # gh-132413: Weakref gets cleared before modules are finalized + code = textwrap.dedent(""" + import os # any module other than sys + + def gen(): + import weakref + wr = weakref.ref(os) + try: + yield + finally: + print( + os is not None, + os is wr() + ) + + it = gen() + next(it) + # quirk: Shutdown starts, then the finally block above follows + """) + res = assert_python_ok('-c', code) + self.assertEqual(res.out.rstrip(), b'True True') + def setUpModule(): global enabled, debug From d7c36b960c675e5772f8cc00f167c71de4fa4f2d Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 8 Jul 2025 23:32:00 +0900 Subject: [PATCH 11/11] add and move test case --- Lib/test/test_gc.py | 23 --------------------- Lib/test/test_weakref.py | 44 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index b0d0c68d6f1dc8..b4cbfb6d774080 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1517,29 +1517,6 @@ def test_ast_fini(self): """) assert_python_ok("-c", code) - def test_weakref_to_module_at_shutdown(self): - # gh-132413: Weakref gets cleared before modules are finalized - code = textwrap.dedent(""" - import os # any module other than sys - - def gen(): - import weakref - wr = weakref.ref(os) - try: - yield - finally: - print( - os is not None, - os is wr() - ) - - it = gen() - next(it) - # quirk: Shutdown starts, then the finally block above follows - """) - res = assert_python_ok('-c', code) - self.assertEqual(res.out.rstrip(), b'True True') - def setUpModule(): global enabled, debug diff --git a/Lib/test/test_weakref.py b/Lib/test/test_weakref.py index 4c7c900eb56ae1..c74c470aaa8693 100644 --- a/Lib/test/test_weakref.py +++ b/Lib/test/test_weakref.py @@ -1044,6 +1044,50 @@ def callback(obj): stderr = res.err.decode("ascii", "backslashreplace") self.assertNotRegex(stderr, "_Py_Dealloc: Deallocator of type 'TestObj'") + def test_module_at_shutdown(self): + # gh-132413: Weakref gets cleared by gc before modules are finalized + code = textwrap.dedent(""" + import os # any module other than sys + import weakref + wr = weakref.ref(os) + + def gen(): + try: + yield + finally: + print( + os is not None, + os is wr() + ) + + it = gen() + next(it) + print('fini', end=' ') + """) + with self.subTest("gen"): + res = script_helper.assert_python_ok('-c', code) + self.assertEqual(res.out.rstrip(), b'fini True True') + + code = textwrap.dedent(""" + import os + import weakref + wr = weakref.ref(os) + + class C: + def __del__(self): + print( + os is not None, + os is wr() + ) + + l = [C()] + l.append(l) + print('fini', end=' ') + """) + with self.subTest("del"): + res = script_helper.assert_python_ok('-c', code) + self.assertEqual(res.out.rstrip(), b'fini True True') + class SubclassableWeakrefTestCase(TestBase):