From a2d90ae65c9251f16de3774d2b80bfa93903c4b2 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 16 Apr 2025 22:50:32 +0900 Subject: [PATCH 01/34] Update datetimetester.py --- Lib/test/datetimetester.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index ecb37250ceb6c4..e2653f38296f0c 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7271,6 +7271,22 @@ def test_update_type_cache(self): """) script_helper.assert_python_ok('-c', script) + def test_module_state_at_shutdown(self): + # gh-132413 + script = textwrap.dedent(""" + import _datetime + + def gen(): + try: + yield + finally: + _datetime.timedelta(days=1) # crash + + it = gen() + next(it) + """) + script_helper.assert_python_ok('-c', script) + def load_tests(loader, standard_tests, pattern): standard_tests.addTest(ZoneInfoCompleteTest()) From 37d317f603ab1067a9c27e73762b1fa1cd6919cc Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 16 Apr 2025 22:53:49 +0900 Subject: [PATCH 02/34] Make interp-dict have a strong ref to the module --- Modules/_datetimemodule.c | 47 +++++++++++++++------------------------ 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 9bba0e3354b26b..2856a1fb74cbee 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -12,6 +12,7 @@ #include "Python.h" #include "pycore_long.h" // _PyLong_GetOne() #include "pycore_object.h" // _PyObject_Init() +#include "pycore_pylifecycle.h" // _Py_IsInterpreterFinalizing() #include "pycore_time.h" // _PyTime_ObjectToTime_t() #include "pycore_unicodeobject.h" // _PyUnicode_Copy() @@ -133,18 +134,13 @@ get_current_module(PyInterpreterState *interp, int *p_reloading) if (dict == NULL) { goto error; } - PyObject *ref = NULL; - if (PyDict_GetItemRef(dict, INTERP_KEY, &ref) < 0) { + if (PyDict_GetItemRef(dict, INTERP_KEY, &mod) < 0) { goto error; } - if (ref != NULL) { + if (mod != NULL) { reloading = 1; - if (ref != Py_None) { - (void)PyWeakref_GetRef(ref, &mod); - if (mod == Py_None) { - Py_CLEAR(mod); - } - Py_DECREF(ref); + if (mod == Py_None) { + mod = NULL; } } if (p_reloading != NULL) { @@ -194,12 +190,7 @@ set_current_module(PyInterpreterState *interp, PyObject *mod) if (dict == NULL) { return -1; } - PyObject *ref = PyWeakref_NewRef(mod, NULL); - if (ref == NULL) { - return -1; - } - int rc = PyDict_SetItem(dict, INTERP_KEY, ref); - Py_DECREF(ref); + int rc = PyDict_SetItem(dict, INTERP_KEY, mod); return rc; } @@ -208,28 +199,26 @@ clear_current_module(PyInterpreterState *interp, PyObject *expected) { PyObject *exc = PyErr_GetRaisedException(); + if (interp->dict == NULL) { + // Do not resurrect a dict during interp-shutdown to avoid the leak + assert(_Py_IsInterpreterFinalizing(interp)); + return; + } + PyObject *dict = PyInterpreterState_GetDict(interp); if (dict == NULL) { goto error; } if (expected != NULL) { - PyObject *ref = NULL; - if (PyDict_GetItemRef(dict, INTERP_KEY, &ref) < 0) { + PyObject *current; + if (PyDict_GetItemRef(dict, INTERP_KEY, ¤t) < 0) { goto error; } - if (ref != NULL) { - PyObject *current = NULL; - int rc = PyWeakref_GetRef(ref, ¤t); - /* We only need "current" for pointer comparison. */ - Py_XDECREF(current); - Py_DECREF(ref); - if (rc < 0) { - goto error; - } - if (current != expected) { - goto finally; - } + /* We only need "current" for pointer comparison. */ + Py_XDECREF(current); + if (current != expected) { + goto finally; } } From 44d09c9b2a6ed13cb7620b5f064d9f8d67fb921c Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Wed, 16 Apr 2025 23:33:36 +0900 Subject: [PATCH 03/34] Fix exc leak --- Modules/_datetimemodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 2856a1fb74cbee..852b8d6bda7a16 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -197,14 +197,14 @@ set_current_module(PyInterpreterState *interp, PyObject *mod) static void clear_current_module(PyInterpreterState *interp, PyObject *expected) { - PyObject *exc = PyErr_GetRaisedException(); - if (interp->dict == NULL) { // Do not resurrect a dict during interp-shutdown to avoid the leak assert(_Py_IsInterpreterFinalizing(interp)); return; } + PyObject *exc = PyErr_GetRaisedException(); + PyObject *dict = PyInterpreterState_GetDict(interp); if (dict == NULL) { goto error; From 19326d4d27561d82c234927149e9ab33a30ad555 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 16 Apr 2025 14:34:07 +0000 Subject: [PATCH 04/34] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2025-04-16-14-34-04.gh-issue-132413.TvpIn2.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2025-04-16-14-34-04.gh-issue-132413.TvpIn2.rst diff --git a/Misc/NEWS.d/next/Library/2025-04-16-14-34-04.gh-issue-132413.TvpIn2.rst b/Misc/NEWS.d/next/Library/2025-04-16-14-34-04.gh-issue-132413.TvpIn2.rst new file mode 100644 index 00000000000000..3e778bf54f8e02 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-04-16-14-34-04.gh-issue-132413.TvpIn2.rst @@ -0,0 +1 @@ +Fix crash in C version of :mod:`datetime` when used during interpreter shutdown. From f805ec71d3a4624ebd76517dfac86cbefc06ef67 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 17 Apr 2025 12:32:31 +0900 Subject: [PATCH 05/34] Add assertion in test --- Lib/test/datetimetester.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index e2653f38296f0c..5d10e60ac5464c 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7269,23 +7269,27 @@ def test_update_type_cache(self): assert isinstance(_datetime.timezone.utc, _datetime.tzinfo) del sys.modules['_datetime'] """) - script_helper.assert_python_ok('-c', script) + res = script_helper.assert_python_ok('-c', script) + self.assertFalse(res.err) def test_module_state_at_shutdown(self): # gh-132413 script = textwrap.dedent(""" + import sys import _datetime def gen(): try: yield finally: - _datetime.timedelta(days=1) # crash + assert not sys.modules + _datetime.timedelta(days=1) it = gen() next(it) """) - script_helper.assert_python_ok('-c', script) + res = script_helper.assert_python_ok('-c', script) + self.assertFalse(res.err) def load_tests(loader, standard_tests, pattern): From 1539cc6ef987e338e054712025f59a3a00e0d17b Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 17 Apr 2025 17:56:59 +0900 Subject: [PATCH 06/34] More assertion --- Lib/test/datetimetester.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 5d10e60ac5464c..200f8489feb020 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7283,7 +7283,8 @@ def gen(): yield finally: assert not sys.modules - _datetime.timedelta(days=1) + td = _datetime.timedelta(days=1) # crash + assert td.days == 1 it = gen() next(it) From e6aa0180908c55554486a8e981d36292c7321459 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 17 Apr 2025 19:09:30 +0900 Subject: [PATCH 07/34] Allow only module's exec() to create interp-dict by using get_current_module() --- Modules/_datetimemodule.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 852b8d6bda7a16..442ec3249b646f 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -159,6 +159,7 @@ static datetime_state * _get_current_state(PyObject **p_mod) { PyInterpreterState *interp = PyInterpreterState_Get(); + assert(interp->dict != NULL); PyObject *mod = get_current_module(interp, NULL); if (mod == NULL) { assert(!PyErr_Occurred()); @@ -185,6 +186,7 @@ _get_current_state(PyObject **p_mod) static int set_current_module(PyInterpreterState *interp, PyObject *mod) { + assert(interp->dict != NULL); assert(mod != NULL); PyObject *dict = PyInterpreterState_GetDict(interp); if (dict == NULL) { From 5141a76fb112b852ac7c3e86baf0931f82737fdb Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 18 Apr 2025 00:39:02 +0900 Subject: [PATCH 08/34] Safer ref cycle between mod and interp-dict --- Modules/_datetimemodule.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 442ec3249b646f..f04c7350dada04 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -50,6 +50,9 @@ typedef struct { /* The interned Unix epoch datetime instance */ PyObject *epoch; + + /* Interpreter's dict holds the module */ + PyObject *interp_dict; } datetime_state; /* The module has a fixed number of static objects, due to being exposed @@ -7276,6 +7279,13 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) return -1; } + PyInterpreterState *interp = PyInterpreterState_Get(); + PyObject *dict = PyInterpreterState_GetDict(interp); + if (dict == NULL) { + return -1; + } + st->interp_dict = Py_NewRef(dict); + return 0; } @@ -7284,6 +7294,7 @@ traverse_state(datetime_state *st, visitproc visit, void *arg) { /* heap types */ Py_VISIT(st->isocalendar_date_type); + Py_VISIT(st->interp_dict); return 0; } @@ -7300,6 +7311,7 @@ clear_state(datetime_state *st) Py_CLEAR(st->us_per_week); Py_CLEAR(st->seconds_per_day); Py_CLEAR(st->epoch); + Py_CLEAR(st->interp_dict); return 0; } @@ -7506,12 +7518,12 @@ module_traverse(PyObject *mod, visitproc visit, void *arg) static int module_clear(PyObject *mod) { - datetime_state *st = get_module_state(mod); - clear_state(st); - PyInterpreterState *interp = PyInterpreterState_Get(); clear_current_module(interp, mod); + datetime_state *st = get_module_state(mod); + clear_state(st); + // The runtime takes care of the static types for us. // See _PyTypes_FiniExtTypes().. From 76cc153533c23c204eedd0e28b049dc1d9ca151c Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 18 Apr 2025 01:58:29 +0900 Subject: [PATCH 09/34] Respect interp-dict held in module state --- Modules/_datetimemodule.c | 44 +++++++++++++++------------------------ 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index f04c7350dada04..20c6b0d73e14a7 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -12,7 +12,6 @@ #include "Python.h" #include "pycore_long.h" // _PyLong_GetOne() #include "pycore_object.h" // _PyObject_Init() -#include "pycore_pylifecycle.h" // _Py_IsInterpreterFinalizing() #include "pycore_time.h" // _PyTime_ObjectToTime_t() #include "pycore_unicodeobject.h" // _PyUnicode_Copy() @@ -162,7 +161,6 @@ static datetime_state * _get_current_state(PyObject **p_mod) { PyInterpreterState *interp = PyInterpreterState_Get(); - assert(interp->dict != NULL); PyObject *mod = get_current_module(interp, NULL); if (mod == NULL) { assert(!PyErr_Occurred()); @@ -187,11 +185,10 @@ _get_current_state(PyObject **p_mod) Py_DECREF(MOD_VAR) static int -set_current_module(PyInterpreterState *interp, PyObject *mod) +set_current_module(datetime_state *st, PyObject *mod) { - assert(interp->dict != NULL); assert(mod != NULL); - PyObject *dict = PyInterpreterState_GetDict(interp); + PyObject *dict = st->interp_dict; if (dict == NULL) { return -1; } @@ -200,19 +197,13 @@ set_current_module(PyInterpreterState *interp, PyObject *mod) } static void -clear_current_module(PyInterpreterState *interp, PyObject *expected) +clear_current_module(datetime_state *st, PyObject *expected) { - if (interp->dict == NULL) { - // Do not resurrect a dict during interp-shutdown to avoid the leak - assert(_Py_IsInterpreterFinalizing(interp)); - return; - } - PyObject *exc = PyErr_GetRaisedException(); - PyObject *dict = PyInterpreterState_GetDict(interp); + PyObject *dict = st->interp_dict; if (dict == NULL) { - goto error; + return; /* Already cleared */ } if (expected != NULL) { @@ -7206,8 +7197,15 @@ create_timezone_from_delta(int days, int sec, int ms, int normalize) */ static int -init_state(datetime_state *st, PyObject *module, PyObject *old_module) +init_state(datetime_state *st, + PyInterpreterState *interp, PyObject *module, PyObject *old_module) { + PyObject *dict = PyInterpreterState_GetDict(interp); + if (dict == NULL) { + return -1; + } + st->interp_dict = Py_NewRef(dict); + /* Each module gets its own heap types. */ #define ADD_TYPE(FIELD, SPEC, BASE) \ do { \ @@ -7226,6 +7224,7 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) assert(old_module != module); datetime_state *st_old = get_module_state(old_module); *st = (datetime_state){ + .interp_dict = st->interp_dict, .isocalendar_date_type = st->isocalendar_date_type, .us_per_ms = Py_NewRef(st_old->us_per_ms), .us_per_second = Py_NewRef(st_old->us_per_second), @@ -7279,13 +7278,6 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) return -1; } - PyInterpreterState *interp = PyInterpreterState_Get(); - PyObject *dict = PyInterpreterState_GetDict(interp); - if (dict == NULL) { - return -1; - } - st->interp_dict = Py_NewRef(dict); - return 0; } @@ -7379,7 +7371,7 @@ _datetime_exec(PyObject *module) } } - if (init_state(st, module, old_module) < 0) { + if (init_state(st, interp, module, old_module) < 0) { goto error; } @@ -7485,7 +7477,7 @@ _datetime_exec(PyObject *module) static_assert(DI100Y == 25 * DI4Y - 1, "DI100Y"); assert(DI100Y == days_before_year(100+1)); - if (set_current_module(interp, module) < 0) { + if (set_current_module(st, module) < 0) { goto error; } @@ -7518,10 +7510,8 @@ module_traverse(PyObject *mod, visitproc visit, void *arg) static int module_clear(PyObject *mod) { - PyInterpreterState *interp = PyInterpreterState_Get(); - clear_current_module(interp, mod); - datetime_state *st = get_module_state(mod); + clear_current_module(st, mod); clear_state(st); // The runtime takes care of the static types for us. From e5bb7c8966fdb6e142c4616febd995af2d1e8b7e Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Mon, 21 Apr 2025 08:01:47 +0900 Subject: [PATCH 10/34] Update _datetimemodule.c --- Modules/_datetimemodule.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 20c6b0d73e14a7..3046bd12ccee3f 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -50,7 +50,8 @@ typedef struct { /* The interned Unix epoch datetime instance */ PyObject *epoch; - /* Interpreter's dict holds the module */ + /* Reference to the interpreter's dict where the current module will be + * reserved even after the referent dict becomes NULL at shutdown. */ PyObject *interp_dict; } datetime_state; @@ -185,7 +186,8 @@ _get_current_state(PyObject **p_mod) Py_DECREF(MOD_VAR) static int -set_current_module(datetime_state *st, PyObject *mod) +set_current_module(datetime_state *st, + PyInterpreterState *interp, PyObject *mod) { assert(mod != NULL); PyObject *dict = st->interp_dict; @@ -199,13 +201,13 @@ set_current_module(datetime_state *st, PyObject *mod) static void clear_current_module(datetime_state *st, PyObject *expected) { - PyObject *exc = PyErr_GetRaisedException(); - PyObject *dict = st->interp_dict; if (dict == NULL) { return; /* Already cleared */ } + PyObject *exc = PyErr_GetRaisedException(); + if (expected != NULL) { PyObject *current; if (PyDict_GetItemRef(dict, INTERP_KEY, ¤t) < 0) { @@ -7284,10 +7286,8 @@ init_state(datetime_state *st, static int traverse_state(datetime_state *st, visitproc visit, void *arg) { - /* heap types */ Py_VISIT(st->isocalendar_date_type); Py_VISIT(st->interp_dict); - return 0; } @@ -7477,7 +7477,7 @@ _datetime_exec(PyObject *module) static_assert(DI100Y == 25 * DI4Y - 1, "DI100Y"); assert(DI100Y == days_before_year(100+1)); - if (set_current_module(st, module) < 0) { + if (set_current_module(st, interp, module) < 0) { goto error; } From 87aa7a23e77081f7eb7e25f79d7e1bc6bc9bc1e1 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Mon, 21 Apr 2025 08:03:42 +0900 Subject: [PATCH 11/34] Update datetimetester.py --- Lib/test/datetimetester.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 200f8489feb020..03b80607ee6713 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7285,6 +7285,7 @@ def gen(): assert not sys.modules td = _datetime.timedelta(days=1) # crash assert td.days == 1 + assert not sys.modules it = gen() next(it) From b425bc860dea77e79efa2eba7d0979ace3e9a401 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 22 Apr 2025 00:14:36 +0900 Subject: [PATCH 12/34] Reword --- Modules/_datetimemodule.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 3046bd12ccee3f..e72ee8f0da2a2c 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -50,8 +50,8 @@ typedef struct { /* The interned Unix epoch datetime instance */ PyObject *epoch; - /* Reference to the interpreter's dict where the current module will be - * reserved even after the referent dict becomes NULL at shutdown. */ + /* Extra reference to the interpreter's dict that will be decref'ed last + /* at shutdown. Use the dict to reserve the current module. */ PyObject *interp_dict; } datetime_state; From 19de232cfea00124ff5e88a70c4076fe06c44582 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 22 Apr 2025 00:21:09 +0900 Subject: [PATCH 13/34] typo --- Modules/_datetimemodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index e72ee8f0da2a2c..d6437562c77fe9 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -51,7 +51,7 @@ typedef struct { PyObject *epoch; /* Extra reference to the interpreter's dict that will be decref'ed last - /* at shutdown. Use the dict to reserve the current module. */ + * at shutdown. Use the dict to reserve the current module. */ PyObject *interp_dict; } datetime_state; From f7b78d9df7ed905968b2763467b86bd213b1d9fd Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 22 Apr 2025 09:13:57 +0900 Subject: [PATCH 14/34] Reword --- Modules/_datetimemodule.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index d6437562c77fe9..83f82a41cb0f8b 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -50,8 +50,9 @@ typedef struct { /* The interned Unix epoch datetime instance */ PyObject *epoch; - /* Extra reference to the interpreter's dict that will be decref'ed last - * at shutdown. Use the dict to reserve the current module. */ + /* Extra reference to the interpreter's dict that will be decref'ed + * last at shutdown. We keep the current module in it, but don't rely + * on PyInterpreterState_GetDict() at the module's final phase. */ PyObject *interp_dict; } datetime_state; From 05811bb27ddc045d79e0c30895c478a571d4b4ed Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 22 Apr 2025 20:45:00 +0900 Subject: [PATCH 15/34] Cleanup --- Modules/_datetimemodule.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 83f82a41cb0f8b..b99bff61b202eb 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -187,16 +187,14 @@ _get_current_state(PyObject **p_mod) Py_DECREF(MOD_VAR) static int -set_current_module(datetime_state *st, - PyInterpreterState *interp, PyObject *mod) +set_current_module(datetime_state *st, PyObject *mod) { assert(mod != NULL); PyObject *dict = st->interp_dict; if (dict == NULL) { return -1; } - int rc = PyDict_SetItem(dict, INTERP_KEY, mod); - return rc; + return PyDict_SetItem(dict, INTERP_KEY, mod); } static void @@ -7200,9 +7198,9 @@ create_timezone_from_delta(int days, int sec, int ms, int normalize) */ static int -init_state(datetime_state *st, - PyInterpreterState *interp, PyObject *module, PyObject *old_module) +init_state(datetime_state *st, PyObject *module, PyObject *old_module) { + PyInterpreterState *interp = PyInterpreterState_Get(); PyObject *dict = PyInterpreterState_GetDict(interp); if (dict == NULL) { return -1; @@ -7372,7 +7370,7 @@ _datetime_exec(PyObject *module) } } - if (init_state(st, interp, module, old_module) < 0) { + if (init_state(st, module, old_module) < 0) { goto error; } @@ -7478,7 +7476,7 @@ _datetime_exec(PyObject *module) static_assert(DI100Y == 25 * DI4Y - 1, "DI100Y"); assert(DI100Y == days_before_year(100+1)); - if (set_current_module(st, interp, module) < 0) { + if (set_current_module(st, module) < 0) { goto error; } From 351ac369b5ee70ac5ce5b16581dd5ceb8d4fb1e4 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 24 Apr 2025 11:04:05 +0900 Subject: [PATCH 16/34] assert(!_Py_IsInterpreterFinalizing()) --- Modules/_datetimemodule.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index b99bff61b202eb..f5782b9ba69a98 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -12,6 +12,7 @@ #include "Python.h" #include "pycore_long.h" // _PyLong_GetOne() #include "pycore_object.h" // _PyObject_Init() +#include "pycore_pylifecycle.h" // _Py_IsInterpreterFinalizing() #include "pycore_time.h" // _PyTime_ObjectToTime_t() #include "pycore_unicodeobject.h" // _PyUnicode_Copy() @@ -35,6 +36,11 @@ static PyTypeObject PyDateTime_TimeZoneType; typedef struct { + /* Extra reference to the interpreter's dict that will be decref'ed + * last at shutdown. We keep the current module in it, but don't rely + * on PyInterpreterState_GetDict() at the module's final phase. */ + PyObject *interp_dict; + /* Module heap types. */ PyTypeObject *isocalendar_date_type; @@ -49,11 +55,6 @@ typedef struct { /* The interned Unix epoch datetime instance */ PyObject *epoch; - - /* Extra reference to the interpreter's dict that will be decref'ed - * last at shutdown. We keep the current module in it, but don't rely - * on PyInterpreterState_GetDict() at the module's final phase. */ - PyObject *interp_dict; } datetime_state; /* The module has a fixed number of static objects, due to being exposed @@ -169,6 +170,7 @@ _get_current_state(PyObject **p_mod) if (PyErr_Occurred()) { return NULL; } + assert(!_Py_IsInterpreterFinalizing(interp)); /* The static types can outlive the module, * so we must re-import the module. */ mod = PyImport_ImportModule("_datetime"); From 0e4a263494728a707147a9fb25781940001be387 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sat, 26 Apr 2025 03:09:28 +0900 Subject: [PATCH 17/34] Add tests --- Lib/test/datetimetester.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 03b80607ee6713..182e1924fee45b 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7293,6 +7293,32 @@ def gen(): res = script_helper.assert_python_ok('-c', script) self.assertFalse(res.err) + def test_remain_only_one_module(self): + script = textwrap.dedent(""" + import sys + import gc + import weakref + ws = weakref.WeakSet() + for _ in range(3): + import _datetime + ws.add(_datetime) + del sys.modules["_datetime"] + del _datetime + gc.collect() + assert len(ws) == 1 + """) + res = script_helper.assert_python_ok('-c', script) + self.assertFalse(res.err) + + @unittest.skipIf(not support.Py_DEBUG, "Debug builds only") + def test_no_leak(self): + script = textwrap.dedent(""" + import datetime + datetime.datetime.strptime('20000101', '%Y%m%d').strftime('%Y%m%d') + """) + res = script_helper.assert_python_ok('-X', 'showrefcount', '-c', script) + self.assertIn(b'[0 refs, 0 blocks]', res.err) + def load_tests(loader, standard_tests, pattern): standard_tests.addTest(ZoneInfoCompleteTest()) From fa0cc4044d38134ef434074701463ed5219df6d2 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sat, 26 Apr 2025 19:50:41 +0900 Subject: [PATCH 18/34] Update tests --- Lib/test/datetimetester.py | 27 ++++++++++++++++++++++++--- Modules/_datetimemodule.c | 14 +------------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 182e1924fee45b..29e84a9f837d20 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7272,7 +7272,7 @@ def test_update_type_cache(self): res = script_helper.assert_python_ok('-c', script) self.assertFalse(res.err) - def test_module_state_at_shutdown(self): + def test_static_type_at_shutdown1(self): # gh-132413 script = textwrap.dedent(""" import sys @@ -7283,7 +7283,27 @@ def gen(): yield finally: assert not sys.modules - td = _datetime.timedelta(days=1) # crash + td = _datetime.timedelta(days=1) + assert td.days == 1 + assert not sys.modules + + it = gen() + next(it) + """) + res = script_helper.assert_python_ok('-c', script) + self.assertFalse(res.err) + + def test_static_type_at_shutdown2(self): + script = textwrap.dedent(""" + import sys + from _datetime import timedelta + + def gen(): + try: + yield + finally: + assert not sys.modules + td = timedelta(days=1) assert td.days == 1 assert not sys.modules @@ -7301,8 +7321,9 @@ def test_remain_only_one_module(self): ws = weakref.WeakSet() for _ in range(3): import _datetime + td = _datetime.timedelta ws.add(_datetime) - del sys.modules["_datetime"] + del sys.modules['_datetime'] del _datetime gc.collect() assert len(ws) == 1 diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index f5782b9ba69a98..d8500e4d32efcb 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -165,19 +165,7 @@ _get_current_state(PyObject **p_mod) { PyInterpreterState *interp = PyInterpreterState_Get(); PyObject *mod = get_current_module(interp, NULL); - if (mod == NULL) { - assert(!PyErr_Occurred()); - if (PyErr_Occurred()) { - return NULL; - } - assert(!_Py_IsInterpreterFinalizing(interp)); - /* The static types can outlive the module, - * so we must re-import the module. */ - mod = PyImport_ImportModule("_datetime"); - if (mod == NULL) { - return NULL; - } - } + assert (mod != NULL); datetime_state *st = get_module_state(mod); *p_mod = mod; return st; From 0377764aa22363776b8ca5984810a7b2507bcc27 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sun, 27 Apr 2025 03:35:19 +0900 Subject: [PATCH 19/34] Take account of interp restart --- Modules/_datetimemodule.c | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index d8500e4d32efcb..b62024c4e0f690 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -165,7 +165,21 @@ _get_current_state(PyObject **p_mod) { PyInterpreterState *interp = PyInterpreterState_Get(); PyObject *mod = get_current_module(interp, NULL); - assert (mod != NULL); + if (mod == NULL) { + assert(!PyErr_Occurred()); + if (PyErr_Occurred()) { + return NULL; + } + /* The static types can outlive the module, + * so we must re-import the module. */ + mod = PyImport_ImportModule("_datetime"); + if (mod == NULL) { + assert(_Py_IsInterpreterFinalizing(interp)); + /* Ask users to take care about the unlikely case + * that happened after restarting the interpreter. */ + return NULL; + } + } datetime_state *st = get_module_state(mod); *p_mod = mod; return st; @@ -2085,6 +2099,9 @@ delta_to_microseconds(PyDateTime_Delta *self) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); + if (current_mod == NULL) { + return NULL; + } x1 = PyLong_FromLong(GET_TD_DAYS(self)); if (x1 == NULL) @@ -2164,6 +2181,9 @@ microseconds_to_delta_ex(PyObject *pyus, PyTypeObject *type) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); + if (current_mod == NULL) { + return NULL; + } tuple = checked_divmod(pyus, CONST_US_PER_SECOND(st)); if (tuple == NULL) { @@ -2749,6 +2769,9 @@ delta_new(PyTypeObject *type, PyObject *args, PyObject *kw) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); + if (current_mod == NULL) { + return NULL; + } /* Argument objects. */ PyObject *day = NULL; @@ -2968,6 +2991,9 @@ delta_total_seconds(PyObject *op, PyObject *Py_UNUSED(dummy)) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); + if (current_mod == NULL) { + return NULL; + } total_seconds = PyNumber_TrueDivide(total_microseconds, CONST_US_PER_SECOND(st)); @@ -3751,6 +3777,9 @@ date_isocalendar(PyObject *self, PyObject *Py_UNUSED(dummy)) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); + if (current_mod == NULL) { + return NULL; + } PyObject *v = iso_calendar_date_new_impl(ISOCALENDAR_DATE_TYPE(st), year, week + 1, day + 1); @@ -6576,6 +6605,9 @@ local_timezone(PyDateTime_DateTime *utc_time) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); + if (current_mod == NULL) { + return NULL; + } delta = datetime_subtract((PyObject *)utc_time, CONST_EPOCH(st)); RELEASE_CURRENT_STATE(st, current_mod); @@ -6820,6 +6852,9 @@ datetime_timestamp(PyObject *op, PyObject *Py_UNUSED(dummy)) if (HASTZINFO(self) && self->tzinfo != Py_None) { PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); + if (current_mod == NULL) { + return NULL; + } PyObject *delta; delta = datetime_subtract(op, CONST_EPOCH(st)); From 9af13a6b4e194c75ef6dbd56fd8bcc76458a1cec Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sun, 27 Apr 2025 10:49:20 +0900 Subject: [PATCH 20/34] Add subinterp tests --- Lib/test/datetimetester.py | 54 ++++++++++++++++++++++++++++++++++++ Modules/_datetimemodule.c | 3 +- Modules/_testcapi/datetime.c | 7 +++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 29e84a9f837d20..25a17a12dc1186 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7313,6 +7313,60 @@ def gen(): res = script_helper.assert_python_ok('-c', script) self.assertFalse(res.err) + def test_static_type_at_shutdown3(self): + script = textwrap.dedent(f''' + import textwrap + from test import support + + subinterp_script = textwrap.dedent(f""" + from _testcapi import get_delta_type + # Test without calling test_datetime_capi() in subinterp + timedelta = get_delta_type() + + def gen(): + try: + yield + finally: + timedelta(days=1) + + it = gen() + next(it) + """) + + import _testcapi + _testcapi.test_datetime_capi() + ret = support.run_in_subinterp(subinterp_script) + assert ret == 0 + ''') + + res = script_helper.assert_python_ok('-c', script) + self.assertIn(b'ImportError: sys.meta_path is None', res.err) + + def test_static_type_before_shutdown(self): + script = textwrap.dedent(f''' + import textwrap + from test import support + + subinterp_script = textwrap.dedent(f""" + from _testcapi import get_delta_type + # Test without calling test_datetime_capi() in subinterp + + import sys + assert '_datetime' not in sys.modules + timedelta = get_delta_type() + timedelta(days=1) + assert '_datetime' in sys.modules + """) + + import _testcapi + _testcapi.test_datetime_capi() + ret = support.run_in_subinterp(subinterp_script) + assert ret == 0 + ''') + + res = script_helper.assert_python_ok('-c', script) + self.assertFalse(res.err) + def test_remain_only_one_module(self): script = textwrap.dedent(""" import sys diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index b62024c4e0f690..cb245683ae570f 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -175,8 +175,7 @@ _get_current_state(PyObject **p_mod) mod = PyImport_ImportModule("_datetime"); if (mod == NULL) { assert(_Py_IsInterpreterFinalizing(interp)); - /* Ask users to take care about the unlikely case - * that happened after restarting the interpreter. */ + /* We do not take care of the unlikely case. */ return NULL; } } diff --git a/Modules/_testcapi/datetime.c b/Modules/_testcapi/datetime.c index b800f9b8eb3473..c62214e88a4bae 100644 --- a/Modules/_testcapi/datetime.c +++ b/Modules/_testcapi/datetime.c @@ -453,6 +453,12 @@ test_PyDateTime_DELTA_GET(PyObject *self, PyObject *obj) return Py_BuildValue("(iii)", days, seconds, microseconds); } +static PyObject * +get_delta_type(PyObject *self, PyObject *args) +{ + return PyDateTimeAPI ? Py_NewRef(PyDateTimeAPI->DeltaType) : Py_None; +} + static PyMethodDef test_methods[] = { {"PyDateTime_DATE_GET", test_PyDateTime_DATE_GET, METH_O}, {"PyDateTime_DELTA_GET", test_PyDateTime_DELTA_GET, METH_O}, @@ -469,6 +475,7 @@ static PyMethodDef test_methods[] = { {"get_datetime_fromdateandtimeandfold", get_datetime_fromdateandtimeandfold, METH_VARARGS}, {"get_datetime_fromtimestamp", get_datetime_fromtimestamp, METH_VARARGS}, {"get_delta_fromdsu", get_delta_fromdsu, METH_VARARGS}, + {"get_delta_type", get_delta_type, METH_NOARGS}, {"get_time_fromtime", get_time_fromtime, METH_VARARGS}, {"get_time_fromtimeandfold", get_time_fromtimeandfold, METH_VARARGS}, {"get_timezone_utc_capi", get_timezone_utc_capi, METH_VARARGS}, From 124d6501bed5f026528433883b94f6d720eb1785 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sun, 27 Apr 2025 15:12:00 +0900 Subject: [PATCH 21/34] Fix tests for free-threaded builds --- Lib/test/datetimetester.py | 98 ++++++++++++++++++++++-------------- Modules/_testcapi/datetime.c | 4 +- 2 files changed, 60 insertions(+), 42 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 25a17a12dc1186..963761c0c0e98e 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7175,6 +7175,7 @@ def test_type_check_in_subinterp(self): spec = importlib.util.spec_from_loader(fullname, loader) module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) + module.test_datetime_capi() def run(type_checker, obj): if not type_checker(obj, True): @@ -7313,58 +7314,77 @@ def gen(): res = script_helper.assert_python_ok('-c', script) self.assertFalse(res.err) - def test_static_type_at_shutdown3(self): - script = textwrap.dedent(f''' + def run_script_132413(self, script): + # iOS requires the use of the custom framework loader, + # not the ExtensionFileLoader. + if sys.platform == "ios": + extension_loader = "AppleFrameworkLoader" + else: + extension_loader = "ExtensionFileLoader" + + main_interp_script = textwrap.dedent(f''' import textwrap from test import support - subinterp_script = textwrap.dedent(f""" - from _testcapi import get_delta_type - # Test without calling test_datetime_capi() in subinterp - timedelta = get_delta_type() - - def gen(): - try: - yield - finally: - timedelta(days=1) - - it = gen() - next(it) + sub_script = textwrap.dedent(""" + if {_interpreters is None}: + import _testcapi as module + else: + import importlib.machinery + import importlib.util + fullname = '_testcapi_datetime' + origin = importlib.util.find_spec('_testcapi').origin + loader = importlib.machinery.{extension_loader}(fullname, origin) + spec = importlib.util.spec_from_loader(fullname, loader) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + + # Skip calling test_datetime_capi() + $REPLACE_ME$ """) import _testcapi _testcapi.test_datetime_capi() - ret = support.run_in_subinterp(subinterp_script) + + if {_interpreters is None}: + ret = support.run_in_subinterp(sub_script) + else: + import _interpreters + config = _interpreters.new_config('isolated').__dict__ + ret = support.run_in_subinterp_with_config(sub_script, **config) + assert ret == 0 - ''') - res = script_helper.assert_python_ok('-c', script) - self.assertIn(b'ImportError: sys.meta_path is None', res.err) + ''').replace('$REPLACE_ME$', textwrap.indent(script, '\x20'*4)) - def test_static_type_before_shutdown(self): - script = textwrap.dedent(f''' - import textwrap - from test import support + res = script_helper.assert_python_ok('-c', main_interp_script) + return res - subinterp_script = textwrap.dedent(f""" - from _testcapi import get_delta_type - # Test without calling test_datetime_capi() in subinterp + def test_static_type_at_shutdown3(self): + script = textwrap.dedent(""" + timedelta = module.get_delta_type() - import sys - assert '_datetime' not in sys.modules - timedelta = get_delta_type() - timedelta(days=1) - assert '_datetime' in sys.modules - """) + def gen(): + try: + yield + finally: + timedelta(days=1) - import _testcapi - _testcapi.test_datetime_capi() - ret = support.run_in_subinterp(subinterp_script) - assert ret == 0 - ''') + it = gen() + next(it) + """) + res = self.run_script_132413(script) + self.assertIn(b'ImportError: sys.meta_path is None', res.err) - res = script_helper.assert_python_ok('-c', script) + def test_static_type_before_shutdown(self): + script = textwrap.dedent(f""" + import sys + assert '_datetime' not in sys.modules + timedelta = module.get_delta_type() + timedelta(days=1) + assert '_datetime' in sys.modules + """) + res = self.run_script_132413(script) self.assertFalse(res.err) def test_remain_only_one_module(self): @@ -7375,7 +7395,7 @@ def test_remain_only_one_module(self): ws = weakref.WeakSet() for _ in range(3): import _datetime - td = _datetime.timedelta + timedelta = _datetime.timedelta ws.add(_datetime) del sys.modules['_datetime'] del _datetime diff --git a/Modules/_testcapi/datetime.c b/Modules/_testcapi/datetime.c index c62214e88a4bae..daf0b8140a382a 100644 --- a/Modules/_testcapi/datetime.c +++ b/Modules/_testcapi/datetime.c @@ -502,9 +502,7 @@ _PyTestCapi_Init_DateTime(PyObject *mod) static int _testcapi_datetime_exec(PyObject *mod) { - if (test_datetime_capi(NULL, NULL) == NULL) { - return -1; - } + // Call test_datetime_capi() in each test. return 0; } From c4905864260eccd12ce62259c28e22f298abec83 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sun, 27 Apr 2025 16:22:32 +0900 Subject: [PATCH 22/34] Nit --- Lib/test/datetimetester.py | 16 ++++++++-------- Modules/_datetimemodule.c | 1 - 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 963761c0c0e98e..9be34441ef273b 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7290,7 +7290,7 @@ def gen(): it = gen() next(it) - """) + """) res = script_helper.assert_python_ok('-c', script) self.assertFalse(res.err) @@ -7310,7 +7310,7 @@ def gen(): it = gen() next(it) - """) + """) res = script_helper.assert_python_ok('-c', script) self.assertFalse(res.err) @@ -7365,10 +7365,10 @@ def test_static_type_at_shutdown3(self): timedelta = module.get_delta_type() def gen(): - try: - yield - finally: - timedelta(days=1) + try: + yield + finally: + timedelta(days=1) it = gen() next(it) @@ -7401,7 +7401,7 @@ def test_remain_only_one_module(self): del _datetime gc.collect() assert len(ws) == 1 - """) + """) res = script_helper.assert_python_ok('-c', script) self.assertFalse(res.err) @@ -7410,7 +7410,7 @@ def test_no_leak(self): script = textwrap.dedent(""" import datetime datetime.datetime.strptime('20000101', '%Y%m%d').strftime('%Y%m%d') - """) + """) res = script_helper.assert_python_ok('-X', 'showrefcount', '-c', script) self.assertIn(b'[0 refs, 0 blocks]', res.err) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index cb245683ae570f..a17b585d97c020 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -175,7 +175,6 @@ _get_current_state(PyObject **p_mod) mod = PyImport_ImportModule("_datetime"); if (mod == NULL) { assert(_Py_IsInterpreterFinalizing(interp)); - /* We do not take care of the unlikely case. */ return NULL; } } From db47683afae91061a22b543158543162cb961c0a Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Mon, 28 Apr 2025 02:24:07 +0900 Subject: [PATCH 23/34] Make tests generic --- Lib/test/datetimetester.py | 139 +++++++++++++++-------------------- Modules/_testcapi/datetime.c | 31 +++++++- 2 files changed, 87 insertions(+), 83 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 2457b007e93405..a1ff0f0989eafe 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7155,7 +7155,10 @@ def test_datetime_from_timestamp(self): self.assertEqual(dt_orig, dt_rt) - def test_type_check_in_subinterp(self): + def assert_python_ok_in_subinterp(self, script, + setup='_testcapi.test_datetime_capi()', + mainsetup='_testcapi.test_datetime_capi()', + config='isolated'): # iOS requires the use of the custom framework loader, # not the ExtensionFileLoader. if sys.platform == "ios": @@ -7163,41 +7166,64 @@ def test_type_check_in_subinterp(self): else: extension_loader = "ExtensionFileLoader" - script = textwrap.dedent(f""" + maincode = textwrap.dedent(f''' + import textwrap + from test import support + + subcode = textwrap.dedent(""" + if {_interpreters is None}: + import _testcapi + else: + import importlib.machinery + import importlib.util + fullname = '_testcapi_datetime' + origin = importlib.util.find_spec('_testcapi').origin + loader = importlib.machinery.{extension_loader}(fullname, origin) + spec = importlib.util.spec_from_loader(fullname, loader) + _testcapi = importlib.util.module_from_spec(spec) + spec.loader.exec_module(_testcapi) + + $SETUP$ + $SCRIPT$ + """) + + import _testcapi + $MAINSETUP$ + if {_interpreters is None}: - import _testcapi as module - module.test_datetime_capi() + ret = support.run_in_subinterp(subcode) else: - import importlib.machinery - import importlib.util - fullname = '_testcapi_datetime' - origin = importlib.util.find_spec('_testcapi').origin - loader = importlib.machinery.{extension_loader}(fullname, origin) - spec = importlib.util.spec_from_loader(fullname, loader) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - module.test_datetime_capi() + import _interpreters + config = _interpreters.new_config('{config}').__dict__ + ret = support.run_in_subinterp_with_config(subcode, **config) + + assert ret == 0 + + ''').replace('$MAINSETUP$', mainsetup) + maincode = maincode.replace('$SETUP$', textwrap.indent(setup, '\x20'*4)) + maincode = maincode.replace('$SCRIPT$', textwrap.indent(script, '\x20'*4)) + + res = script_helper.assert_python_ok('-c', maincode) + return res + def test_type_check_in_subinterp(self): + script = textwrap.dedent(f""" def run(type_checker, obj): if not type_checker(obj, True): raise TypeError(f'{{type(obj)}} is not C API type') import _datetime - run(module.datetime_check_date, _datetime.date.today()) - run(module.datetime_check_datetime, _datetime.datetime.now()) - run(module.datetime_check_time, _datetime.time(12, 30)) - run(module.datetime_check_delta, _datetime.timedelta(1)) - run(module.datetime_check_tzinfo, _datetime.tzinfo()) + run(_testcapi.datetime_check_date, _datetime.date.today()) + run(_testcapi.datetime_check_datetime, _datetime.datetime.now()) + run(_testcapi.datetime_check_time, _datetime.time(12, 30)) + run(_testcapi.datetime_check_delta, _datetime.timedelta(1)) + run(_testcapi.datetime_check_tzinfo, _datetime.tzinfo()) """) - if _interpreters is None: - ret = support.run_in_subinterp(script) - self.assertEqual(ret, 0) - else: - for name in ('isolated', 'legacy'): - with self.subTest(name): - config = _interpreters.new_config(name).__dict__ - ret = support.run_in_subinterp_with_config(script, **config) - self.assertEqual(ret, 0) + self.assert_python_ok_in_subinterp(script, mainsetup='') + if _interpreters is not None: + with self.subTest('legacy'): + self.assert_python_ok_in_subinterp(script, mainsetup='', + config='legacy') class ExtensionModuleTests(unittest.TestCase): @@ -7284,6 +7310,7 @@ def gen(): try: yield finally: + # Exceptions are ignored here assert not sys.modules td = _datetime.timedelta(days=1) assert td.days == 1 @@ -7315,55 +7342,9 @@ def gen(): res = script_helper.assert_python_ok('-c', script) self.assertFalse(res.err) - def run_script_132413(self, script): - # iOS requires the use of the custom framework loader, - # not the ExtensionFileLoader. - if sys.platform == "ios": - extension_loader = "AppleFrameworkLoader" - else: - extension_loader = "ExtensionFileLoader" - - main_interp_script = textwrap.dedent(f''' - import textwrap - from test import support - - sub_script = textwrap.dedent(""" - if {_interpreters is None}: - import _testcapi as module - else: - import importlib.machinery - import importlib.util - fullname = '_testcapi_datetime' - origin = importlib.util.find_spec('_testcapi').origin - loader = importlib.machinery.{extension_loader}(fullname, origin) - spec = importlib.util.spec_from_loader(fullname, loader) - module = importlib.util.module_from_spec(spec) - spec.loader.exec_module(module) - - # Skip calling test_datetime_capi() - $REPLACE_ME$ - """) - - import _testcapi - _testcapi.test_datetime_capi() - - if {_interpreters is None}: - ret = support.run_in_subinterp(sub_script) - else: - import _interpreters - config = _interpreters.new_config('isolated').__dict__ - ret = support.run_in_subinterp_with_config(sub_script, **config) - - assert ret == 0 - - ''').replace('$REPLACE_ME$', textwrap.indent(script, '\x20'*4)) - - res = script_helper.assert_python_ok('-c', main_interp_script) - return res - def test_static_type_at_shutdown3(self): script = textwrap.dedent(""" - timedelta = module.get_delta_type() + timedelta = _testcapi.get_capi_types()['timedelta'] def gen(): try: @@ -7374,19 +7355,18 @@ def gen(): it = gen() next(it) """) - res = self.run_script_132413(script) + res = CapiTest.assert_python_ok_in_subinterp(self, script, setup='') self.assertIn(b'ImportError: sys.meta_path is None', res.err) def test_static_type_before_shutdown(self): script = textwrap.dedent(f""" import sys assert '_datetime' not in sys.modules - timedelta = module.get_delta_type() + timedelta = _testcapi.get_capi_types()['timedelta'] timedelta(days=1) assert '_datetime' in sys.modules """) - res = self.run_script_132413(script) - self.assertFalse(res.err) + CapiTest.assert_python_ok_in_subinterp(self, script, setup='') def test_remain_only_one_module(self): script = textwrap.dedent(""" @@ -7403,8 +7383,7 @@ def test_remain_only_one_module(self): gc.collect() assert len(ws) == 1 """) - res = script_helper.assert_python_ok('-c', script) - self.assertFalse(res.err) + script_helper.assert_python_ok('-c', script) @unittest.skipIf(not support.Py_DEBUG, "Debug builds only") def test_no_leak(self): diff --git a/Modules/_testcapi/datetime.c b/Modules/_testcapi/datetime.c index daf0b8140a382a..f7d693d1c61850 100644 --- a/Modules/_testcapi/datetime.c +++ b/Modules/_testcapi/datetime.c @@ -454,9 +454,34 @@ test_PyDateTime_DELTA_GET(PyObject *self, PyObject *obj) } static PyObject * -get_delta_type(PyObject *self, PyObject *args) +get_capi_types(PyObject *self, PyObject *args) { - return PyDateTimeAPI ? Py_NewRef(PyDateTimeAPI->DeltaType) : Py_None; + if (PyDateTimeAPI == NULL) { + Py_RETURN_NONE; + } + PyObject *dict = PyDict_New(); + if (dict == NULL) { + return NULL; + } + if (PyDict_SetItemString(dict, "date", PyDateTimeAPI->DateType) < 0) { + goto error; + } + if (PyDict_SetItemString(dict, "time", PyDateTimeAPI->TimeType) < 0) { + goto error; + } + if (PyDict_SetItemString(dict, "datetime", PyDateTimeAPI->DateTimeType) < 0) { + goto error; + } + if (PyDict_SetItemString(dict, "timedelta", PyDateTimeAPI->DeltaType) < 0) { + goto error; + } + if (PyDict_SetItemString(dict, "tzinfo", PyDateTimeAPI->TZInfoType) < 0) { + goto error; + } + return dict; +error: + Py_DECREF(dict); + return NULL; } static PyMethodDef test_methods[] = { @@ -475,11 +500,11 @@ static PyMethodDef test_methods[] = { {"get_datetime_fromdateandtimeandfold", get_datetime_fromdateandtimeandfold, METH_VARARGS}, {"get_datetime_fromtimestamp", get_datetime_fromtimestamp, METH_VARARGS}, {"get_delta_fromdsu", get_delta_fromdsu, METH_VARARGS}, - {"get_delta_type", get_delta_type, METH_NOARGS}, {"get_time_fromtime", get_time_fromtime, METH_VARARGS}, {"get_time_fromtimeandfold", get_time_fromtimeandfold, METH_VARARGS}, {"get_timezone_utc_capi", get_timezone_utc_capi, METH_VARARGS}, {"get_timezones_offset_zero", get_timezones_offset_zero, METH_NOARGS}, + {"get_capi_types", get_capi_types, METH_NOARGS}, {"make_timezones_capi", make_timezones_capi, METH_NOARGS}, {"test_datetime_capi", test_datetime_capi, METH_NOARGS}, {NULL}, From ddd1635e370edbbeddb83e94f895b107194ceade Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Mon, 28 Apr 2025 02:36:19 +0900 Subject: [PATCH 24/34] Fix warnings --- Modules/_testcapi/datetime.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Modules/_testcapi/datetime.c b/Modules/_testcapi/datetime.c index f7d693d1c61850..0b4a968d0a0d0c 100644 --- a/Modules/_testcapi/datetime.c +++ b/Modules/_testcapi/datetime.c @@ -463,19 +463,19 @@ get_capi_types(PyObject *self, PyObject *args) if (dict == NULL) { return NULL; } - if (PyDict_SetItemString(dict, "date", PyDateTimeAPI->DateType) < 0) { + if (PyDict_SetItemString(dict, "date", (PyObject *)PyDateTimeAPI->DateType) < 0) { goto error; } - if (PyDict_SetItemString(dict, "time", PyDateTimeAPI->TimeType) < 0) { + if (PyDict_SetItemString(dict, "time", (PyObject *)PyDateTimeAPI->TimeType) < 0) { goto error; } - if (PyDict_SetItemString(dict, "datetime", PyDateTimeAPI->DateTimeType) < 0) { + if (PyDict_SetItemString(dict, "datetime", (PyObject *)PyDateTimeAPI->DateTimeType) < 0) { goto error; } - if (PyDict_SetItemString(dict, "timedelta", PyDateTimeAPI->DeltaType) < 0) { + if (PyDict_SetItemString(dict, "timedelta", (PyObject *)PyDateTimeAPI->DeltaType) < 0) { goto error; } - if (PyDict_SetItemString(dict, "tzinfo", PyDateTimeAPI->TZInfoType) < 0) { + if (PyDict_SetItemString(dict, "tzinfo", (PyObject *)PyDateTimeAPI->TZInfoType) < 0) { goto error; } return dict; From 746bf8541f9bb2f4c0bcda0aed1aac8fd235b95c Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 1 May 2025 04:55:19 +0900 Subject: [PATCH 25/34] Update tests --- Lib/test/datetimetester.py | 223 +++++++++++++++++++---------------- Lib/test/test_embed.py | 28 +++++ Modules/_datetimemodule.c | 2 - Modules/_testcapi/datetime.c | 26 +++- 4 files changed, 172 insertions(+), 107 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index a1ff0f0989eafe..520d8bf80ea6e0 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7155,10 +7155,8 @@ def test_datetime_from_timestamp(self): self.assertEqual(dt_orig, dt_rt) - def assert_python_ok_in_subinterp(self, script, - setup='_testcapi.test_datetime_capi()', - mainsetup='_testcapi.test_datetime_capi()', - config='isolated'): + def assert_python_ok_in_subinterp(self, script, init='', fini='', + repeat=1, config='isolated'): # iOS requires the use of the custom framework loader, # not the ExtensionFileLoader. if sys.platform == "ios": @@ -7166,44 +7164,44 @@ def assert_python_ok_in_subinterp(self, script, else: extension_loader = "ExtensionFileLoader" - maincode = textwrap.dedent(f''' - import textwrap - from test import support - - subcode = textwrap.dedent(""" - if {_interpreters is None}: - import _testcapi - else: - import importlib.machinery - import importlib.util - fullname = '_testcapi_datetime' - origin = importlib.util.find_spec('_testcapi').origin - loader = importlib.machinery.{extension_loader}(fullname, origin) - spec = importlib.util.spec_from_loader(fullname, loader) - _testcapi = importlib.util.module_from_spec(spec) - spec.loader.exec_module(_testcapi) - - $SETUP$ - $SCRIPT$ - """) - - import _testcapi - $MAINSETUP$ - + code = textwrap.dedent(f''' + subinterp_code = """ if {_interpreters is None}: - ret = support.run_in_subinterp(subcode) + import _testcapi else: - import _interpreters - config = _interpreters.new_config('{config}').__dict__ - ret = support.run_in_subinterp_with_config(subcode, **config) - - assert ret == 0 + import importlib.machinery + import importlib.util + fullname = '_testcapi_datetime' + origin = importlib.util.find_spec('_testcapi').origin + loader = importlib.machinery.{extension_loader}(fullname, origin) + spec = importlib.util.spec_from_loader(fullname, loader) + _testcapi = importlib.util.module_from_spec(spec) + spec.loader.exec_module(_testcapi) + interp_index = $INTERP_INDEX$ + setup = _testcapi.test_datetime_capi_newinterp # call it if needed + $SCRIPT$ + """ - ''').replace('$MAINSETUP$', mainsetup) - maincode = maincode.replace('$SETUP$', textwrap.indent(setup, '\x20'*4)) - maincode = maincode.replace('$SCRIPT$', textwrap.indent(script, '\x20'*4)) + import _testcapi + from test import support + setup = _testcapi.test_datetime_capi_newinterp + $INIT$ - res = script_helper.assert_python_ok('-c', maincode) + for idx in range({repeat}): + subcode = subinterp_code.replace('$INTERP_INDEX$', str(idx)) + if {_interpreters is None}: + ret = support.run_in_subinterp(subcode) + else: + import _interpreters + config = _interpreters.new_config('{config}').__dict__ + ret = support.run_in_subinterp_with_config(subcode, **config) + assert ret == 0 + $FINI$ + ''') + code = code.replace('$INIT$', init).replace('$FINI$', fini) + code = code.replace('$SCRIPT$', script) + + res = script_helper.assert_python_ok('-c', code) return res def test_type_check_in_subinterp(self): @@ -7212,18 +7210,18 @@ def run(type_checker, obj): if not type_checker(obj, True): raise TypeError(f'{{type(obj)}} is not C API type') + setup() import _datetime run(_testcapi.datetime_check_date, _datetime.date.today()) run(_testcapi.datetime_check_datetime, _datetime.datetime.now()) run(_testcapi.datetime_check_time, _datetime.time(12, 30)) run(_testcapi.datetime_check_delta, _datetime.timedelta(1)) run(_testcapi.datetime_check_tzinfo, _datetime.tzinfo()) - """) - self.assert_python_ok_in_subinterp(script, mainsetup='') + """) + self.assert_python_ok_in_subinterp(script) if _interpreters is not None: - with self.subTest('legacy'): - self.assert_python_ok_in_subinterp(script, mainsetup='', - config='legacy') + with self.subTest(name := 'legacy'): + self.assert_python_ok_in_subinterp(script, config=name) class ExtensionModuleTests(unittest.TestCase): @@ -7232,6 +7230,9 @@ def setUp(self): if self.__class__.__name__.endswith('Pure'): self.skipTest('Not relevant in pure Python') + def assert_python_ok_in_subinterp(self, *args, **kwargs): + return CapiTest.assert_python_ok_in_subinterp(self, *args, **kwargs) + @support.cpython_only def test_gh_120161(self): with self.subTest('simple'): @@ -7300,11 +7301,65 @@ def test_update_type_cache(self): res = script_helper.assert_python_ok('-c', script) self.assertFalse(res.err) - def test_static_type_at_shutdown1(self): + def test_module_free(self): + script = textwrap.dedent(""" + import sys + import gc + import weakref + ws = weakref.WeakSet() + for _ in range(3): + import _datetime + timedelta = _datetime.timedelta # static type + ws.add(_datetime) + del sys.modules['_datetime'] + del _datetime + gc.collect() + assert len(ws) == 1 # only one remains + """) + script_helper.assert_python_ok('-c', script) + + @unittest.skipIf(not support.Py_DEBUG, "Debug builds only") + def test_no_leak(self): + script = textwrap.dedent(""" + import datetime + datetime.datetime.strptime('20000101', '%Y%m%d').strftime('%Y%m%d') + """) + res = script_helper.assert_python_ok('-X', 'showrefcount', '-c', script) + self.assertIn(b'[0 refs, 0 blocks]', res.err) + + def test_static_type_on_subinterp(self): + script = textwrap.dedent(""" + date = _testcapi.get_capi_types()['date'] + date.today + """) + with_setup = 'setup()' + script + with self.subTest('[PyDateTime_IMPORT] main: no, sub: yes'): + self.assert_python_ok_in_subinterp(with_setup) + + with self.subTest('[PyDateTime_IMPORT] main: yes, sub: yes'): + # Fails if the setup() means test_datetime_capi() rather than + # test_datetime_capi_newinterp() + self.assert_python_ok_in_subinterp(with_setup, 'setup()') + self.assert_python_ok_in_subinterp('setup()', fini=with_setup) + self.assert_python_ok_in_subinterp(with_setup, repeat=2) + + with_import = 'import _datetime' + script + with self.subTest('Explicit import'): + self.assert_python_ok_in_subinterp(with_import, 'setup()') + + with_import = textwrap.dedent(""" + timedelta = _testcapi.get_capi_types()['timedelta'] + timedelta(days=1) + """) + script + with self.subTest('Implicit import'): + self.assert_python_ok_in_subinterp(with_import, 'setup()') + + def test_static_type_at_shutdown(self): # gh-132413 script = textwrap.dedent(""" import sys import _datetime + timedelta = _datetime.timedelta def gen(): try: @@ -7314,23 +7369,29 @@ def gen(): assert not sys.modules td = _datetime.timedelta(days=1) assert td.days == 1 + td = timedelta(days=1) + assert td.days == 1 assert not sys.modules it = gen() next(it) - """) - res = script_helper.assert_python_ok('-c', script) - self.assertFalse(res.err) + """) + with self.subTest('MainInterpreter'): + res = script_helper.assert_python_ok('-c', script) + self.assertFalse(res.err) + with self.subTest('Subinterpreter'): + res = self.assert_python_ok_in_subinterp(script) + self.assertFalse(res.err) - def test_static_type_at_shutdown2(self): script = textwrap.dedent(""" import sys - from _datetime import timedelta + timedelta = _testcapi.get_capi_types()['timedelta'] def gen(): try: yield finally: + # Exceptions are ignored here assert not sys.modules td = timedelta(days=1) assert td.days == 1 @@ -7338,61 +7399,15 @@ def gen(): it = gen() next(it) - """) - res = script_helper.assert_python_ok('-c', script) - self.assertFalse(res.err) - - def test_static_type_at_shutdown3(self): - script = textwrap.dedent(""" - timedelta = _testcapi.get_capi_types()['timedelta'] - - def gen(): - try: - yield - finally: - timedelta(days=1) - - it = gen() - next(it) - """) - res = CapiTest.assert_python_ok_in_subinterp(self, script, setup='') - self.assertIn(b'ImportError: sys.meta_path is None', res.err) - - def test_static_type_before_shutdown(self): - script = textwrap.dedent(f""" - import sys - assert '_datetime' not in sys.modules - timedelta = _testcapi.get_capi_types()['timedelta'] - timedelta(days=1) - assert '_datetime' in sys.modules - """) - CapiTest.assert_python_ok_in_subinterp(self, script, setup='') - - def test_remain_only_one_module(self): - script = textwrap.dedent(""" - import sys - import gc - import weakref - ws = weakref.WeakSet() - for _ in range(3): - import _datetime - timedelta = _datetime.timedelta - ws.add(_datetime) - del sys.modules['_datetime'] - del _datetime - gc.collect() - assert len(ws) == 1 - """) - script_helper.assert_python_ok('-c', script) - - @unittest.skipIf(not support.Py_DEBUG, "Debug builds only") - def test_no_leak(self): - script = textwrap.dedent(""" - import datetime - datetime.datetime.strptime('20000101', '%Y%m%d').strftime('%Y%m%d') - """) - res = script_helper.assert_python_ok('-X', 'showrefcount', '-c', script) - self.assertIn(b'[0 refs, 0 blocks]', res.err) + """) + with self.subTest('[PyDateTime_IMPORT] main: yes, sub: no'): + res = self.assert_python_ok_in_subinterp(script, 'setup()') + self.assertIn(b'ImportError: sys.meta_path is None', res.err) + + with_import = 'setup()' + script + with self.subTest('[PyDateTime_IMPORT] main: no, sub: yes'): + res = self.assert_python_ok_in_subinterp(with_import) + self.assertFalse(res.err) def load_tests(loader, standard_tests, pattern): diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index e06e684408ca6b..d272e106d66b10 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -440,6 +440,34 @@ def test_datetime_reset_strptime(self): out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) self.assertEqual(out, '20000101\n' * INIT_LOOPS) + def test_datetime_capi_at_shutdown(self): + # gh-132413: datetime module is currently tested in an interp's life. + # PyDateTime_IMPORT needs to be called at least once after the restart. + code = textwrap.dedent(""" + import sys + import _testcapi + _testcapi.test_datetime_capi_newinterp() + timedelta = _testcapi.get_capi_types()['timedelta'] + + def gen(): + try: + yield + finally: + assert not sys.modules + res = 0 + try: + timedelta(days=1) + res = 1 + except ImportError: + res = 2 + print(res) + + it = gen() + next(it) + """) + out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) + self.assertEqual(out, '1\n' * INIT_LOOPS) + def test_static_types_inherited_slots(self): script = textwrap.dedent(""" import test.support diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index a17b585d97c020..0af5c1f0ce6a1e 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -12,7 +12,6 @@ #include "Python.h" #include "pycore_long.h" // _PyLong_GetOne() #include "pycore_object.h" // _PyObject_Init() -#include "pycore_pylifecycle.h" // _Py_IsInterpreterFinalizing() #include "pycore_time.h" // _PyTime_ObjectToTime_t() #include "pycore_unicodeobject.h" // _PyUnicode_Copy() @@ -174,7 +173,6 @@ _get_current_state(PyObject **p_mod) * so we must re-import the module. */ mod = PyImport_ImportModule("_datetime"); if (mod == NULL) { - assert(_Py_IsInterpreterFinalizing(interp)); return NULL; } } diff --git a/Modules/_testcapi/datetime.c b/Modules/_testcapi/datetime.c index 0b4a968d0a0d0c..375196e28fb727 100644 --- a/Modules/_testcapi/datetime.c +++ b/Modules/_testcapi/datetime.c @@ -35,6 +35,29 @@ test_datetime_capi(PyObject *self, PyObject *args) Py_RETURN_NONE; } +static PyObject * +test_datetime_capi_newinterp(PyObject *self, PyObject *args) +{ + // Call PyDateTime_IMPORT at least once in each interpreter's life + if (PyDateTimeAPI != NULL && test_run_counter == 0) { + PyErr_SetString(PyExc_AssertionError, + "PyDateTime_CAPI somehow initialized"); + return NULL; + } + test_run_counter++; + PyDateTime_IMPORT; + + if (PyDateTimeAPI == NULL) { + return NULL; + } + assert(!PyType_HasFeature(PyDateTimeAPI->DateType, Py_TPFLAGS_HEAPTYPE)); + assert(!PyType_HasFeature(PyDateTimeAPI->TimeType, Py_TPFLAGS_HEAPTYPE)); + assert(!PyType_HasFeature(PyDateTimeAPI->DateTimeType, Py_TPFLAGS_HEAPTYPE)); + assert(!PyType_HasFeature(PyDateTimeAPI->DeltaType, Py_TPFLAGS_HEAPTYPE)); + assert(!PyType_HasFeature(PyDateTimeAPI->TZInfoType, Py_TPFLAGS_HEAPTYPE)); + Py_RETURN_NONE; +} + /* Functions exposing the C API type checking for testing */ #define MAKE_DATETIME_CHECK_FUNC(check_method, exact_method) \ do { \ @@ -507,6 +530,7 @@ static PyMethodDef test_methods[] = { {"get_capi_types", get_capi_types, METH_NOARGS}, {"make_timezones_capi", make_timezones_capi, METH_NOARGS}, {"test_datetime_capi", test_datetime_capi, METH_NOARGS}, + {"test_datetime_capi_newinterp",test_datetime_capi_newinterp, METH_NOARGS}, {NULL}, }; @@ -527,7 +551,7 @@ _PyTestCapi_Init_DateTime(PyObject *mod) static int _testcapi_datetime_exec(PyObject *mod) { - // Call test_datetime_capi() in each test. + // The execution does not invoke PyDateTime_IMPORT return 0; } From a8f76faad85906c08ef6df2ecb1b605a7d67e31c Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 8 May 2025 07:18:40 +0900 Subject: [PATCH 26/34] Revert to original (main) --- Lib/test/datetimetester.py | 179 ++++------------------------------- Lib/test/test_embed.py | 28 ------ Modules/_datetimemodule.c | 99 +++++++++---------- Modules/_testcapi/datetime.c | 60 +----------- 4 files changed, 67 insertions(+), 299 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 520d8bf80ea6e0..55844ec35a90c9 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7155,8 +7155,7 @@ def test_datetime_from_timestamp(self): self.assertEqual(dt_orig, dt_rt) - def assert_python_ok_in_subinterp(self, script, init='', fini='', - repeat=1, config='isolated'): + def test_type_check_in_subinterp(self): # iOS requires the use of the custom framework loader, # not the ExtensionFileLoader. if sys.platform == "ios": @@ -7164,10 +7163,10 @@ def assert_python_ok_in_subinterp(self, script, init='', fini='', else: extension_loader = "ExtensionFileLoader" - code = textwrap.dedent(f''' - subinterp_code = """ + script = textwrap.dedent(f""" if {_interpreters is None}: - import _testcapi + import _testcapi as module + module.test_datetime_capi() else: import importlib.machinery import importlib.util @@ -7175,53 +7174,29 @@ def assert_python_ok_in_subinterp(self, script, init='', fini='', origin = importlib.util.find_spec('_testcapi').origin loader = importlib.machinery.{extension_loader}(fullname, origin) spec = importlib.util.spec_from_loader(fullname, loader) - _testcapi = importlib.util.module_from_spec(spec) - spec.loader.exec_module(_testcapi) - interp_index = $INTERP_INDEX$ - setup = _testcapi.test_datetime_capi_newinterp # call it if needed - $SCRIPT$ - """ + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) - import _testcapi - from test import support - setup = _testcapi.test_datetime_capi_newinterp - $INIT$ - - for idx in range({repeat}): - subcode = subinterp_code.replace('$INTERP_INDEX$', str(idx)) - if {_interpreters is None}: - ret = support.run_in_subinterp(subcode) - else: - import _interpreters - config = _interpreters.new_config('{config}').__dict__ - ret = support.run_in_subinterp_with_config(subcode, **config) - assert ret == 0 - $FINI$ - ''') - code = code.replace('$INIT$', init).replace('$FINI$', fini) - code = code.replace('$SCRIPT$', script) - - res = script_helper.assert_python_ok('-c', code) - return res - - def test_type_check_in_subinterp(self): - script = textwrap.dedent(f""" def run(type_checker, obj): if not type_checker(obj, True): raise TypeError(f'{{type(obj)}} is not C API type') - setup() import _datetime - run(_testcapi.datetime_check_date, _datetime.date.today()) - run(_testcapi.datetime_check_datetime, _datetime.datetime.now()) - run(_testcapi.datetime_check_time, _datetime.time(12, 30)) - run(_testcapi.datetime_check_delta, _datetime.timedelta(1)) - run(_testcapi.datetime_check_tzinfo, _datetime.tzinfo()) - """) - self.assert_python_ok_in_subinterp(script) - if _interpreters is not None: - with self.subTest(name := 'legacy'): - self.assert_python_ok_in_subinterp(script, config=name) + run(module.datetime_check_date, _datetime.date.today()) + run(module.datetime_check_datetime, _datetime.datetime.now()) + run(module.datetime_check_time, _datetime.time(12, 30)) + run(module.datetime_check_delta, _datetime.timedelta(1)) + run(module.datetime_check_tzinfo, _datetime.tzinfo()) + """) + if _interpreters is None: + ret = support.run_in_subinterp(script) + self.assertEqual(ret, 0) + else: + for name in ('isolated', 'legacy'): + with self.subTest(name): + config = _interpreters.new_config(name).__dict__ + ret = support.run_in_subinterp_with_config(script, **config) + self.assertEqual(ret, 0) class ExtensionModuleTests(unittest.TestCase): @@ -7230,9 +7205,6 @@ def setUp(self): if self.__class__.__name__.endswith('Pure'): self.skipTest('Not relevant in pure Python') - def assert_python_ok_in_subinterp(self, *args, **kwargs): - return CapiTest.assert_python_ok_in_subinterp(self, *args, **kwargs) - @support.cpython_only def test_gh_120161(self): with self.subTest('simple'): @@ -7298,117 +7270,8 @@ def test_update_type_cache(self): assert isinstance(_datetime.timezone.utc, _datetime.tzinfo) del sys.modules['_datetime'] """) - res = script_helper.assert_python_ok('-c', script) - self.assertFalse(res.err) - - def test_module_free(self): - script = textwrap.dedent(""" - import sys - import gc - import weakref - ws = weakref.WeakSet() - for _ in range(3): - import _datetime - timedelta = _datetime.timedelta # static type - ws.add(_datetime) - del sys.modules['_datetime'] - del _datetime - gc.collect() - assert len(ws) == 1 # only one remains - """) script_helper.assert_python_ok('-c', script) - @unittest.skipIf(not support.Py_DEBUG, "Debug builds only") - def test_no_leak(self): - script = textwrap.dedent(""" - import datetime - datetime.datetime.strptime('20000101', '%Y%m%d').strftime('%Y%m%d') - """) - res = script_helper.assert_python_ok('-X', 'showrefcount', '-c', script) - self.assertIn(b'[0 refs, 0 blocks]', res.err) - - def test_static_type_on_subinterp(self): - script = textwrap.dedent(""" - date = _testcapi.get_capi_types()['date'] - date.today - """) - with_setup = 'setup()' + script - with self.subTest('[PyDateTime_IMPORT] main: no, sub: yes'): - self.assert_python_ok_in_subinterp(with_setup) - - with self.subTest('[PyDateTime_IMPORT] main: yes, sub: yes'): - # Fails if the setup() means test_datetime_capi() rather than - # test_datetime_capi_newinterp() - self.assert_python_ok_in_subinterp(with_setup, 'setup()') - self.assert_python_ok_in_subinterp('setup()', fini=with_setup) - self.assert_python_ok_in_subinterp(with_setup, repeat=2) - - with_import = 'import _datetime' + script - with self.subTest('Explicit import'): - self.assert_python_ok_in_subinterp(with_import, 'setup()') - - with_import = textwrap.dedent(""" - timedelta = _testcapi.get_capi_types()['timedelta'] - timedelta(days=1) - """) + script - with self.subTest('Implicit import'): - self.assert_python_ok_in_subinterp(with_import, 'setup()') - - def test_static_type_at_shutdown(self): - # gh-132413 - script = textwrap.dedent(""" - import sys - import _datetime - timedelta = _datetime.timedelta - - def gen(): - try: - yield - finally: - # Exceptions are ignored here - assert not sys.modules - td = _datetime.timedelta(days=1) - assert td.days == 1 - td = timedelta(days=1) - assert td.days == 1 - assert not sys.modules - - it = gen() - next(it) - """) - with self.subTest('MainInterpreter'): - res = script_helper.assert_python_ok('-c', script) - self.assertFalse(res.err) - with self.subTest('Subinterpreter'): - res = self.assert_python_ok_in_subinterp(script) - self.assertFalse(res.err) - - script = textwrap.dedent(""" - import sys - timedelta = _testcapi.get_capi_types()['timedelta'] - - def gen(): - try: - yield - finally: - # Exceptions are ignored here - assert not sys.modules - td = timedelta(days=1) - assert td.days == 1 - assert not sys.modules - - it = gen() - next(it) - """) - with self.subTest('[PyDateTime_IMPORT] main: yes, sub: no'): - res = self.assert_python_ok_in_subinterp(script, 'setup()') - self.assertIn(b'ImportError: sys.meta_path is None', res.err) - - with_import = 'setup()' + script - with self.subTest('[PyDateTime_IMPORT] main: no, sub: yes'): - res = self.assert_python_ok_in_subinterp(with_import) - self.assertFalse(res.err) - def load_tests(loader, standard_tests, pattern): standard_tests.addTest(ZoneInfoCompleteTest()) diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index d272e106d66b10..e06e684408ca6b 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -440,34 +440,6 @@ def test_datetime_reset_strptime(self): out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) self.assertEqual(out, '20000101\n' * INIT_LOOPS) - def test_datetime_capi_at_shutdown(self): - # gh-132413: datetime module is currently tested in an interp's life. - # PyDateTime_IMPORT needs to be called at least once after the restart. - code = textwrap.dedent(""" - import sys - import _testcapi - _testcapi.test_datetime_capi_newinterp() - timedelta = _testcapi.get_capi_types()['timedelta'] - - def gen(): - try: - yield - finally: - assert not sys.modules - res = 0 - try: - timedelta(days=1) - res = 1 - except ImportError: - res = 2 - print(res) - - it = gen() - next(it) - """) - out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) - self.assertEqual(out, '1\n' * INIT_LOOPS) - def test_static_types_inherited_slots(self): script = textwrap.dedent(""" import test.support diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 0af5c1f0ce6a1e..9bba0e3354b26b 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -35,11 +35,6 @@ static PyTypeObject PyDateTime_TimeZoneType; typedef struct { - /* Extra reference to the interpreter's dict that will be decref'ed - * last at shutdown. We keep the current module in it, but don't rely - * on PyInterpreterState_GetDict() at the module's final phase. */ - PyObject *interp_dict; - /* Module heap types. */ PyTypeObject *isocalendar_date_type; @@ -138,13 +133,18 @@ get_current_module(PyInterpreterState *interp, int *p_reloading) if (dict == NULL) { goto error; } - if (PyDict_GetItemRef(dict, INTERP_KEY, &mod) < 0) { + PyObject *ref = NULL; + if (PyDict_GetItemRef(dict, INTERP_KEY, &ref) < 0) { goto error; } - if (mod != NULL) { + if (ref != NULL) { reloading = 1; - if (mod == Py_None) { - mod = NULL; + if (ref != Py_None) { + (void)PyWeakref_GetRef(ref, &mod); + if (mod == Py_None) { + Py_CLEAR(mod); + } + Py_DECREF(ref); } } if (p_reloading != NULL) { @@ -187,35 +187,49 @@ _get_current_state(PyObject **p_mod) Py_DECREF(MOD_VAR) static int -set_current_module(datetime_state *st, PyObject *mod) +set_current_module(PyInterpreterState *interp, PyObject *mod) { assert(mod != NULL); - PyObject *dict = st->interp_dict; + PyObject *dict = PyInterpreterState_GetDict(interp); if (dict == NULL) { return -1; } - return PyDict_SetItem(dict, INTERP_KEY, mod); + PyObject *ref = PyWeakref_NewRef(mod, NULL); + if (ref == NULL) { + return -1; + } + int rc = PyDict_SetItem(dict, INTERP_KEY, ref); + Py_DECREF(ref); + return rc; } static void -clear_current_module(datetime_state *st, PyObject *expected) +clear_current_module(PyInterpreterState *interp, PyObject *expected) { - PyObject *dict = st->interp_dict; + PyObject *exc = PyErr_GetRaisedException(); + + PyObject *dict = PyInterpreterState_GetDict(interp); if (dict == NULL) { - return; /* Already cleared */ + goto error; } - PyObject *exc = PyErr_GetRaisedException(); - if (expected != NULL) { - PyObject *current; - if (PyDict_GetItemRef(dict, INTERP_KEY, ¤t) < 0) { + PyObject *ref = NULL; + if (PyDict_GetItemRef(dict, INTERP_KEY, &ref) < 0) { goto error; } - /* We only need "current" for pointer comparison. */ - Py_XDECREF(current); - if (current != expected) { - goto finally; + if (ref != NULL) { + PyObject *current = NULL; + int rc = PyWeakref_GetRef(ref, ¤t); + /* We only need "current" for pointer comparison. */ + Py_XDECREF(current); + Py_DECREF(ref); + if (rc < 0) { + goto error; + } + if (current != expected) { + goto finally; + } } } @@ -2095,9 +2109,6 @@ delta_to_microseconds(PyDateTime_Delta *self) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - if (current_mod == NULL) { - return NULL; - } x1 = PyLong_FromLong(GET_TD_DAYS(self)); if (x1 == NULL) @@ -2177,9 +2188,6 @@ microseconds_to_delta_ex(PyObject *pyus, PyTypeObject *type) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - if (current_mod == NULL) { - return NULL; - } tuple = checked_divmod(pyus, CONST_US_PER_SECOND(st)); if (tuple == NULL) { @@ -2765,9 +2773,6 @@ delta_new(PyTypeObject *type, PyObject *args, PyObject *kw) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - if (current_mod == NULL) { - return NULL; - } /* Argument objects. */ PyObject *day = NULL; @@ -2987,9 +2992,6 @@ delta_total_seconds(PyObject *op, PyObject *Py_UNUSED(dummy)) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - if (current_mod == NULL) { - return NULL; - } total_seconds = PyNumber_TrueDivide(total_microseconds, CONST_US_PER_SECOND(st)); @@ -3773,9 +3775,6 @@ date_isocalendar(PyObject *self, PyObject *Py_UNUSED(dummy)) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - if (current_mod == NULL) { - return NULL; - } PyObject *v = iso_calendar_date_new_impl(ISOCALENDAR_DATE_TYPE(st), year, week + 1, day + 1); @@ -6601,9 +6600,6 @@ local_timezone(PyDateTime_DateTime *utc_time) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - if (current_mod == NULL) { - return NULL; - } delta = datetime_subtract((PyObject *)utc_time, CONST_EPOCH(st)); RELEASE_CURRENT_STATE(st, current_mod); @@ -6848,9 +6844,6 @@ datetime_timestamp(PyObject *op, PyObject *Py_UNUSED(dummy)) if (HASTZINFO(self) && self->tzinfo != Py_None) { PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - if (current_mod == NULL) { - return NULL; - } PyObject *delta; delta = datetime_subtract(op, CONST_EPOCH(st)); @@ -7221,13 +7214,6 @@ create_timezone_from_delta(int days, int sec, int ms, int normalize) static int init_state(datetime_state *st, PyObject *module, PyObject *old_module) { - PyInterpreterState *interp = PyInterpreterState_Get(); - PyObject *dict = PyInterpreterState_GetDict(interp); - if (dict == NULL) { - return -1; - } - st->interp_dict = Py_NewRef(dict); - /* Each module gets its own heap types. */ #define ADD_TYPE(FIELD, SPEC, BASE) \ do { \ @@ -7246,7 +7232,6 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) assert(old_module != module); datetime_state *st_old = get_module_state(old_module); *st = (datetime_state){ - .interp_dict = st->interp_dict, .isocalendar_date_type = st->isocalendar_date_type, .us_per_ms = Py_NewRef(st_old->us_per_ms), .us_per_second = Py_NewRef(st_old->us_per_second), @@ -7306,8 +7291,9 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) static int traverse_state(datetime_state *st, visitproc visit, void *arg) { + /* heap types */ Py_VISIT(st->isocalendar_date_type); - Py_VISIT(st->interp_dict); + return 0; } @@ -7323,7 +7309,6 @@ clear_state(datetime_state *st) Py_CLEAR(st->us_per_week); Py_CLEAR(st->seconds_per_day); Py_CLEAR(st->epoch); - Py_CLEAR(st->interp_dict); return 0; } @@ -7497,7 +7482,7 @@ _datetime_exec(PyObject *module) static_assert(DI100Y == 25 * DI4Y - 1, "DI100Y"); assert(DI100Y == days_before_year(100+1)); - if (set_current_module(st, module) < 0) { + if (set_current_module(interp, module) < 0) { goto error; } @@ -7531,9 +7516,11 @@ static int module_clear(PyObject *mod) { datetime_state *st = get_module_state(mod); - clear_current_module(st, mod); clear_state(st); + PyInterpreterState *interp = PyInterpreterState_Get(); + clear_current_module(interp, mod); + // The runtime takes care of the static types for us. // See _PyTypes_FiniExtTypes().. diff --git a/Modules/_testcapi/datetime.c b/Modules/_testcapi/datetime.c index 375196e28fb727..b800f9b8eb3473 100644 --- a/Modules/_testcapi/datetime.c +++ b/Modules/_testcapi/datetime.c @@ -35,29 +35,6 @@ test_datetime_capi(PyObject *self, PyObject *args) Py_RETURN_NONE; } -static PyObject * -test_datetime_capi_newinterp(PyObject *self, PyObject *args) -{ - // Call PyDateTime_IMPORT at least once in each interpreter's life - if (PyDateTimeAPI != NULL && test_run_counter == 0) { - PyErr_SetString(PyExc_AssertionError, - "PyDateTime_CAPI somehow initialized"); - return NULL; - } - test_run_counter++; - PyDateTime_IMPORT; - - if (PyDateTimeAPI == NULL) { - return NULL; - } - assert(!PyType_HasFeature(PyDateTimeAPI->DateType, Py_TPFLAGS_HEAPTYPE)); - assert(!PyType_HasFeature(PyDateTimeAPI->TimeType, Py_TPFLAGS_HEAPTYPE)); - assert(!PyType_HasFeature(PyDateTimeAPI->DateTimeType, Py_TPFLAGS_HEAPTYPE)); - assert(!PyType_HasFeature(PyDateTimeAPI->DeltaType, Py_TPFLAGS_HEAPTYPE)); - assert(!PyType_HasFeature(PyDateTimeAPI->TZInfoType, Py_TPFLAGS_HEAPTYPE)); - Py_RETURN_NONE; -} - /* Functions exposing the C API type checking for testing */ #define MAKE_DATETIME_CHECK_FUNC(check_method, exact_method) \ do { \ @@ -476,37 +453,6 @@ test_PyDateTime_DELTA_GET(PyObject *self, PyObject *obj) return Py_BuildValue("(iii)", days, seconds, microseconds); } -static PyObject * -get_capi_types(PyObject *self, PyObject *args) -{ - if (PyDateTimeAPI == NULL) { - Py_RETURN_NONE; - } - PyObject *dict = PyDict_New(); - if (dict == NULL) { - return NULL; - } - if (PyDict_SetItemString(dict, "date", (PyObject *)PyDateTimeAPI->DateType) < 0) { - goto error; - } - if (PyDict_SetItemString(dict, "time", (PyObject *)PyDateTimeAPI->TimeType) < 0) { - goto error; - } - if (PyDict_SetItemString(dict, "datetime", (PyObject *)PyDateTimeAPI->DateTimeType) < 0) { - goto error; - } - if (PyDict_SetItemString(dict, "timedelta", (PyObject *)PyDateTimeAPI->DeltaType) < 0) { - goto error; - } - if (PyDict_SetItemString(dict, "tzinfo", (PyObject *)PyDateTimeAPI->TZInfoType) < 0) { - goto error; - } - return dict; -error: - Py_DECREF(dict); - return NULL; -} - static PyMethodDef test_methods[] = { {"PyDateTime_DATE_GET", test_PyDateTime_DATE_GET, METH_O}, {"PyDateTime_DELTA_GET", test_PyDateTime_DELTA_GET, METH_O}, @@ -527,10 +473,8 @@ static PyMethodDef test_methods[] = { {"get_time_fromtimeandfold", get_time_fromtimeandfold, METH_VARARGS}, {"get_timezone_utc_capi", get_timezone_utc_capi, METH_VARARGS}, {"get_timezones_offset_zero", get_timezones_offset_zero, METH_NOARGS}, - {"get_capi_types", get_capi_types, METH_NOARGS}, {"make_timezones_capi", make_timezones_capi, METH_NOARGS}, {"test_datetime_capi", test_datetime_capi, METH_NOARGS}, - {"test_datetime_capi_newinterp",test_datetime_capi_newinterp, METH_NOARGS}, {NULL}, }; @@ -551,7 +495,9 @@ _PyTestCapi_Init_DateTime(PyObject *mod) static int _testcapi_datetime_exec(PyObject *mod) { - // The execution does not invoke PyDateTime_IMPORT + if (test_datetime_capi(NULL, NULL) == NULL) { + return -1; + } return 0; } From c7fc55e5b1ec73c2fc26ffe0cc9a606fd1599916 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 8 May 2025 07:52:55 +0900 Subject: [PATCH 27/34] Convert IsoCalendarDate to static type (3.12) --- Lib/test/datetimetester.py | 52 ++++++++++++ Lib/test/test_embed.py | 42 ++++++++++ Modules/_datetimemodule.c | 149 ++++++++++++++++++----------------- Modules/_testcapi/datetime.c | 32 ++++++++ 4 files changed, 201 insertions(+), 74 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 55844ec35a90c9..5acbaa733748f5 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7270,8 +7270,60 @@ def test_update_type_cache(self): assert isinstance(_datetime.timezone.utc, _datetime.tzinfo) del sys.modules['_datetime'] """) + res = script_helper.assert_python_ok('-c', script) + self.assertFalse(res.err) + + def test_module_free(self): + script = textwrap.dedent(""" + import sys + import gc + import weakref + ws = weakref.WeakSet() + for _ in range(3): + import _datetime + timedelta = _datetime.timedelta # static type + ws.add(_datetime) + del sys.modules['_datetime'] + del _datetime + gc.collect() + assert len(ws) == 0 + """) script_helper.assert_python_ok('-c', script) + @unittest.skipIf(not support.Py_DEBUG, "Debug builds only") + def test_no_leak(self): + script = textwrap.dedent(""" + import datetime + datetime.datetime.strptime('20000101', '%Y%m%d').strftime('%Y%m%d') + """) + res = script_helper.assert_python_ok('-X', 'showrefcount', '-c', script) + self.assertIn(b'[0 refs, 0 blocks]', res.err) + + def test_static_type_at_shutdown(self): + # gh-132413 + script = textwrap.dedent(""" + import sys + import _datetime + timedelta = _datetime.timedelta + + def gen(): + try: + yield + finally: + # Exceptions are ignored here + assert not sys.modules + td = _datetime.timedelta(days=1) + assert td.days == 1 + td = timedelta(days=1) + assert td.days == 1 + assert not sys.modules + + it = gen() + next(it) + """) + res = script_helper.assert_python_ok('-c', script) + self.assertFalse(res.err) + def load_tests(loader, standard_tests, pattern): standard_tests.addTest(ZoneInfoCompleteTest()) diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index e06e684408ca6b..35a2e5e36a5882 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -440,6 +440,48 @@ def test_datetime_reset_strptime(self): out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) self.assertEqual(out, '20000101\n' * INIT_LOOPS) + def test_datetime_capi_type_address(self): + # Check if the C-API types keep their addresses until runtime shutdown + code = textwrap.dedent(""" + import _datetime as d + print( + f'{id(d.date)}' + f'{id(d.time)}' + f'{id(d.datetime)}' + f'{id(d.timedelta)}' + f'{id(d.tzinfo)}' + ) + """) + out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) + self.assertEqual(len(set(out.splitlines())), 1) + + def test_datetime_capi_at_shutdown(self): + # gh-132413 + code = textwrap.dedent(""" + import sys + import _testcapi + _testcapi.test_datetime_capi() # PyDateTime_IMPORT only once + timedelta = _testcapi.get_capi_types()['timedelta'] + + def gen(): + try: + yield + finally: + assert not sys.modules + res = 0 + try: + timedelta(days=1) + res = 1 + except ImportError: + res = 2 + print(res) + + it = gen() + next(it) + """) + out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) + self.assertEqual(out, '1\n' * INIT_LOOPS) + def test_static_types_inherited_slots(self): script = textwrap.dedent(""" import test.support diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 9bba0e3354b26b..a3099ac9927809 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -32,12 +32,10 @@ static PyTypeObject PyDateTime_TimeType; static PyTypeObject PyDateTime_DeltaType; static PyTypeObject PyDateTime_TZInfoType; static PyTypeObject PyDateTime_TimeZoneType; +static PyTypeObject PyDateTime_IsoCalendarDateType; typedef struct { - /* Module heap types. */ - PyTypeObject *isocalendar_date_type; - /* Conversion factors. */ PyObject *us_per_ms; // 1_000 PyObject *us_per_second; // 1_000_000 @@ -75,7 +73,7 @@ typedef struct { #define DELTA_TYPE(st) &PyDateTime_DeltaType #define TZINFO_TYPE(st) &PyDateTime_TZInfoType #define TIMEZONE_TYPE(st) &PyDateTime_TimeZoneType -#define ISOCALENDAR_DATE_TYPE(st) st->isocalendar_date_type +#define ISOCALENDAR_DATE_TYPE(st) &PyDateTime_IsoCalendarDateType #define PyDate_CAST(op) ((PyDateTime_Date *)(op)) #define PyDate_Check(op) PyObject_TypeCheck(op, DATE_TYPE(NO_STATE)) @@ -102,15 +100,52 @@ typedef struct { #define PyIsoCalendarDate_CAST(op) ((PyDateTime_IsoCalendarDate *)(op)) -#define CONST_US_PER_MS(st) st->us_per_ms -#define CONST_US_PER_SECOND(st) st->us_per_second -#define CONST_US_PER_MINUTE(st) st->us_per_minute -#define CONST_US_PER_HOUR(st) st->us_per_hour -#define CONST_US_PER_DAY(st) st->us_per_day -#define CONST_US_PER_WEEK(st) st->us_per_week -#define CONST_SEC_PER_DAY(st) st->seconds_per_day -#define CONST_EPOCH(st) st->epoch +static inline PyObject * +get_const_us_per_ms(datetime_state *st) { + return st ? st->us_per_ms : PyLong_FromLong(1000); +} + +static inline PyObject * +get_const_us_per_second(datetime_state *st) { + return st ? st->us_per_second : PyLong_FromLong(1000000); +} + +static inline PyObject * +get_const_us_per_minute(datetime_state *st) { + return st ? st->us_per_minute : PyLong_FromLong(60000000); +} + +static inline PyObject * +get_const_us_per_hour(datetime_state *st) { + return st ? st->us_per_hour : PyLong_FromDouble(3600000000.0); +} + +static inline PyObject * +get_const_us_per_day(datetime_state *st) { + return st ? st->us_per_day : PyLong_FromDouble(86400000000.0); +} + +static inline PyObject * +get_const_us_per_week(datetime_state *st) { + return st ? st->us_per_week : PyLong_FromDouble(604800000000.0); +} + +static inline PyObject * +get_const_sec_per_day(datetime_state *st) { + return st ? st->seconds_per_day : PyLong_FromLong(24 * 3600); +} + +#define CONST_US_PER_MS(st) get_const_us_per_ms(st) +#define CONST_US_PER_SECOND(st) get_const_us_per_second(st) +#define CONST_US_PER_MINUTE(st) get_const_us_per_minute(st) +#define CONST_US_PER_HOUR(st) get_const_us_per_hour(st) +#define CONST_US_PER_DAY(st) get_const_us_per_day(st) +#define CONST_US_PER_WEEK(st) get_const_us_per_week(st) +#define CONST_SEC_PER_DAY(st) get_const_sec_per_day(st) #define CONST_UTC(st) ((PyObject *)&utc_timezone) +#define CONST_EPOCH(st) \ + (st ? ((datetime_state *)st)->epoch \ + : new_datetime(1970, 1, 1, 0, 0, 0, 0, (PyObject *)&utc_timezone, 0)) static datetime_state * get_module_state(PyObject *module) @@ -173,6 +208,7 @@ _get_current_state(PyObject **p_mod) * so we must re-import the module. */ mod = PyImport_ImportModule("_datetime"); if (mod == NULL) { + PyErr_Clear(); return NULL; } } @@ -184,7 +220,7 @@ _get_current_state(PyObject **p_mod) #define GET_CURRENT_STATE(MOD_VAR) \ _get_current_state(&MOD_VAR) #define RELEASE_CURRENT_STATE(ST_VAR, MOD_VAR) \ - Py_DECREF(MOD_VAR) + Py_XDECREF(MOD_VAR) static int set_current_module(PyInterpreterState *interp, PyObject *mod) @@ -3691,40 +3727,19 @@ static PyMethodDef iso_calendar_date_methods[] = { {NULL, NULL}, }; -static int -iso_calendar_date_traverse(PyObject *self, visitproc visit, void *arg) -{ - Py_VISIT(Py_TYPE(self)); - return PyTuple_Type.tp_traverse(self, visit, arg); -} - -static void -iso_calendar_date_dealloc(PyObject *self) -{ - PyTypeObject *tp = Py_TYPE(self); - PyTuple_Type.tp_dealloc(self); // delegate GC-untrack as well - Py_DECREF(tp); -} - -static PyType_Slot isocal_slots[] = { - {Py_tp_repr, iso_calendar_date_repr}, - {Py_tp_doc, (void *)iso_calendar_date__doc__}, - {Py_tp_methods, iso_calendar_date_methods}, - {Py_tp_getset, iso_calendar_date_getset}, - {Py_tp_new, iso_calendar_date_new}, - {Py_tp_dealloc, iso_calendar_date_dealloc}, - {Py_tp_traverse, iso_calendar_date_traverse}, - {0, NULL}, +static PyTypeObject PyDateTime_IsoCalendarDateType = { + PyVarObject_HEAD_INIT(NULL, 0) + .tp_name = "datetime.IsoCalendarDate", + .tp_basicsize = sizeof(PyDateTime_IsoCalendarDate), + .tp_repr = (reprfunc) iso_calendar_date_repr, + .tp_flags = Py_TPFLAGS_DEFAULT, + .tp_doc = iso_calendar_date__doc__, + .tp_methods = iso_calendar_date_methods, + .tp_getset = iso_calendar_date_getset, + // .tp_base = &PyTuple_Type, // filled in PyInit__datetime + .tp_new = iso_calendar_date_new, }; -static PyType_Spec isocal_spec = { - .name = "datetime.IsoCalendarDate", - .basicsize = sizeof(PyDateTime_IsoCalendarDate), - .flags = (Py_TPFLAGS_DEFAULT | - Py_TPFLAGS_HAVE_GC | - Py_TPFLAGS_IMMUTABLETYPE), - .slots = isocal_slots, -}; /*[clinic input] @classmethod @@ -3776,7 +3791,7 @@ date_isocalendar(PyObject *self, PyObject *Py_UNUSED(dummy)) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - PyObject *v = iso_calendar_date_new_impl(ISOCALENDAR_DATE_TYPE(st), + PyObject *v = iso_calendar_date_new_impl(&PyDateTime_IsoCalendarDateType, year, week + 1, day + 1); RELEASE_CURRENT_STATE(st, current_mod); if (v == NULL) { @@ -7155,6 +7170,8 @@ static PyTypeObject * const capi_types[] = { &PyDateTime_TZInfoType, /* Indirectly, via the utc object. */ &PyDateTime_TimeZoneType, + /* Not exposed */ + &PyDateTime_IsoCalendarDateType, }; /* The C-API is process-global. This violates interpreter isolation @@ -7214,25 +7231,10 @@ create_timezone_from_delta(int days, int sec, int ms, int normalize) static int init_state(datetime_state *st, PyObject *module, PyObject *old_module) { - /* Each module gets its own heap types. */ -#define ADD_TYPE(FIELD, SPEC, BASE) \ - do { \ - PyObject *cls = PyType_FromModuleAndSpec( \ - module, SPEC, (PyObject *)BASE); \ - if (cls == NULL) { \ - return -1; \ - } \ - st->FIELD = (PyTypeObject *)cls; \ - } while (0) - - ADD_TYPE(isocalendar_date_type, &isocal_spec, &PyTuple_Type); -#undef ADD_TYPE - if (old_module != NULL) { assert(old_module != module); datetime_state *st_old = get_module_state(old_module); *st = (datetime_state){ - .isocalendar_date_type = st->isocalendar_date_type, .us_per_ms = Py_NewRef(st_old->us_per_ms), .us_per_second = Py_NewRef(st_old->us_per_second), .us_per_minute = Py_NewRef(st_old->us_per_minute), @@ -7245,19 +7247,19 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) return 0; } - st->us_per_ms = PyLong_FromLong(1000); + st->us_per_ms = CONST_US_PER_MS(NULL); if (st->us_per_ms == NULL) { return -1; } - st->us_per_second = PyLong_FromLong(1000000); + st->us_per_second = CONST_US_PER_SECOND(NULL); if (st->us_per_second == NULL) { return -1; } - st->us_per_minute = PyLong_FromLong(60000000); + st->us_per_minute = CONST_US_PER_MINUTE(NULL); if (st->us_per_minute == NULL) { return -1; } - st->seconds_per_day = PyLong_FromLong(24 * 3600); + st->seconds_per_day = CONST_SEC_PER_DAY(NULL); if (st->seconds_per_day == NULL) { return -1; } @@ -7265,22 +7267,21 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) /* The rest are too big for 32-bit ints, but even * us_per_week fits in 40 bits, so doubles should be exact. */ - st->us_per_hour = PyLong_FromDouble(3600000000.0); + st->us_per_hour = CONST_US_PER_HOUR(NULL); if (st->us_per_hour == NULL) { return -1; } - st->us_per_day = PyLong_FromDouble(86400000000.0); + st->us_per_day = CONST_US_PER_DAY(NULL); if (st->us_per_day == NULL) { return -1; } - st->us_per_week = PyLong_FromDouble(604800000000.0); + st->us_per_week = CONST_US_PER_WEEK(NULL); if (st->us_per_week == NULL) { return -1; } /* Init Unix epoch */ - st->epoch = new_datetime( - 1970, 1, 1, 0, 0, 0, 0, (PyObject *)&utc_timezone, 0); + st->epoch = CONST_EPOCH(NULL); if (st->epoch == NULL) { return -1; } @@ -7291,16 +7292,12 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) static int traverse_state(datetime_state *st, visitproc visit, void *arg) { - /* heap types */ - Py_VISIT(st->isocalendar_date_type); - return 0; } static int clear_state(datetime_state *st) { - Py_CLEAR(st->isocalendar_date_type); Py_CLEAR(st->us_per_ms); Py_CLEAR(st->us_per_second); Py_CLEAR(st->us_per_minute); @@ -7323,6 +7320,7 @@ init_static_types(PyInterpreterState *interp, int reloading) // `&...` is not a constant expression according to a strict reading // of C standards. Fill tp_base at run-time rather than statically. // See https://bugs.python.org/issue40777 + PyDateTime_IsoCalendarDateType.tp_base = &PyTuple_Type; PyDateTime_TimeZoneType.tp_base = &PyDateTime_TZInfoType; PyDateTime_DateTimeType.tp_base = &PyDateTime_DateType; @@ -7369,6 +7367,9 @@ _datetime_exec(PyObject *module) for (size_t i = 0; i < Py_ARRAY_LENGTH(capi_types); i++) { PyTypeObject *type = capi_types[i]; + if (type == &PyDateTime_IsoCalendarDateType) { + continue; + } const char *name = _PyType_Name(type); assert(name != NULL); if (PyModule_AddObjectRef(module, name, (PyObject *)type) < 0) { diff --git a/Modules/_testcapi/datetime.c b/Modules/_testcapi/datetime.c index b800f9b8eb3473..36fd9e50ca154c 100644 --- a/Modules/_testcapi/datetime.c +++ b/Modules/_testcapi/datetime.c @@ -453,6 +453,37 @@ test_PyDateTime_DELTA_GET(PyObject *self, PyObject *obj) return Py_BuildValue("(iii)", days, seconds, microseconds); } +static PyObject * +get_capi_types(PyObject *self, PyObject *args) +{ + if (PyDateTimeAPI == NULL) { + Py_RETURN_NONE; + } + PyObject *dict = PyDict_New(); + if (dict == NULL) { + return NULL; + } + if (PyDict_SetItemString(dict, "date", (PyObject *)PyDateTimeAPI->DateType) < 0) { + goto error; + } + if (PyDict_SetItemString(dict, "time", (PyObject *)PyDateTimeAPI->TimeType) < 0) { + goto error; + } + if (PyDict_SetItemString(dict, "datetime", (PyObject *)PyDateTimeAPI->DateTimeType) < 0) { + goto error; + } + if (PyDict_SetItemString(dict, "timedelta", (PyObject *)PyDateTimeAPI->DeltaType) < 0) { + goto error; + } + if (PyDict_SetItemString(dict, "tzinfo", (PyObject *)PyDateTimeAPI->TZInfoType) < 0) { + goto error; + } + return dict; +error: + Py_DECREF(dict); + return NULL; +} + static PyMethodDef test_methods[] = { {"PyDateTime_DATE_GET", test_PyDateTime_DATE_GET, METH_O}, {"PyDateTime_DELTA_GET", test_PyDateTime_DELTA_GET, METH_O}, @@ -473,6 +504,7 @@ static PyMethodDef test_methods[] = { {"get_time_fromtimeandfold", get_time_fromtimeandfold, METH_VARARGS}, {"get_timezone_utc_capi", get_timezone_utc_capi, METH_VARARGS}, {"get_timezones_offset_zero", get_timezones_offset_zero, METH_NOARGS}, + {"get_capi_types", get_capi_types, METH_NOARGS}, {"make_timezones_capi", make_timezones_capi, METH_NOARGS}, {"test_datetime_capi", test_datetime_capi, METH_NOARGS}, {NULL}, From 539cfed5f1390ac0a22fe7d1926337bdc8425c6c Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 8 May 2025 08:09:22 +0900 Subject: [PATCH 28/34] Fix warning --- Modules/_datetimemodule.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index a3099ac9927809..b6b5edb92fafd4 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -3788,12 +3788,8 @@ date_isocalendar(PyObject *self, PyObject *Py_UNUSED(dummy)) week = 0; } - PyObject *current_mod = NULL; - datetime_state *st = GET_CURRENT_STATE(current_mod); - PyObject *v = iso_calendar_date_new_impl(&PyDateTime_IsoCalendarDateType, year, week + 1, day + 1); - RELEASE_CURRENT_STATE(st, current_mod); if (v == NULL) { return NULL; } From d9f85448e706d7c06fc333731bd25ff1563c5c6d Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 8 May 2025 13:39:08 +0900 Subject: [PATCH 29/34] Fix refleaks --- Lib/test/datetimetester.py | 5 + Modules/_datetimemodule.c | 219 +++++++++++++++++++++++++------------ 2 files changed, 153 insertions(+), 71 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 5acbaa733748f5..2406a1f7ad13bb 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7324,6 +7324,11 @@ def gen(): res = script_helper.assert_python_ok('-c', script) self.assertFalse(res.err) + if support.Py_DEBUG: + with self.subTest('Refleak'): + res = script_helper.assert_python_ok('-X', 'showrefcount', '-c', script) + self.assertIn(b'[0 refs, 0 blocks]', res.err) + def load_tests(loader, standard_tests, pattern): standard_tests.addTest(ZoneInfoCompleteTest()) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index b6b5edb92fafd4..f0f1cfeaeb1017 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -100,54 +100,17 @@ typedef struct { #define PyIsoCalendarDate_CAST(op) ((PyDateTime_IsoCalendarDate *)(op)) -static inline PyObject * -get_const_us_per_ms(datetime_state *st) { - return st ? st->us_per_ms : PyLong_FromLong(1000); -} - -static inline PyObject * -get_const_us_per_second(datetime_state *st) { - return st ? st->us_per_second : PyLong_FromLong(1000000); -} - -static inline PyObject * -get_const_us_per_minute(datetime_state *st) { - return st ? st->us_per_minute : PyLong_FromLong(60000000); -} - -static inline PyObject * -get_const_us_per_hour(datetime_state *st) { - return st ? st->us_per_hour : PyLong_FromDouble(3600000000.0); -} - -static inline PyObject * -get_const_us_per_day(datetime_state *st) { - return st ? st->us_per_day : PyLong_FromDouble(86400000000.0); -} - -static inline PyObject * -get_const_us_per_week(datetime_state *st) { - return st ? st->us_per_week : PyLong_FromDouble(604800000000.0); -} - -static inline PyObject * -get_const_sec_per_day(datetime_state *st) { - return st ? st->seconds_per_day : PyLong_FromLong(24 * 3600); -} - -#define CONST_US_PER_MS(st) get_const_us_per_ms(st) -#define CONST_US_PER_SECOND(st) get_const_us_per_second(st) -#define CONST_US_PER_MINUTE(st) get_const_us_per_minute(st) -#define CONST_US_PER_HOUR(st) get_const_us_per_hour(st) -#define CONST_US_PER_DAY(st) get_const_us_per_day(st) -#define CONST_US_PER_WEEK(st) get_const_us_per_week(st) -#define CONST_SEC_PER_DAY(st) get_const_sec_per_day(st) #define CONST_UTC(st) ((PyObject *)&utc_timezone) -#define CONST_EPOCH(st) \ - (st ? ((datetime_state *)st)->epoch \ - : new_datetime(1970, 1, 1, 0, 0, 0, 0, (PyObject *)&utc_timezone, 0)) - -static datetime_state * +static inline PyObject *get_const_us_per_ms(datetime_state *st); +static inline PyObject *get_const_us_per_second(datetime_state *st); +static inline PyObject *get_const_us_per_minute(datetime_state *st); +static inline PyObject *get_const_us_per_hour(datetime_state *st); +static inline PyObject *get_const_us_per_day(datetime_state *st); +static inline PyObject *get_const_us_per_week(datetime_state *st); +static inline PyObject *get_const_sec_per_day(datetime_state *st); +static inline PyObject *get_const_epoch(datetime_state *st); + +static inline datetime_state * get_module_state(PyObject *module) { void *state = _PyModule_GetState(module); @@ -2149,7 +2112,12 @@ delta_to_microseconds(PyDateTime_Delta *self) x1 = PyLong_FromLong(GET_TD_DAYS(self)); if (x1 == NULL) goto Done; - x2 = PyNumber_Multiply(x1, CONST_SEC_PER_DAY(st)); /* days in seconds */ + PyObject *sec_per_day = get_const_sec_per_day(st); + if (sec_per_day == NULL) { + goto Done; + } + x2 = PyNumber_Multiply(x1, sec_per_day); /* days in seconds */ + Py_DECREF(sec_per_day); if (x2 == NULL) goto Done; Py_SETREF(x1, NULL); @@ -2166,7 +2134,12 @@ delta_to_microseconds(PyDateTime_Delta *self) /* x1 = */ x2 = NULL; /* x3 has days+seconds in seconds */ - x1 = PyNumber_Multiply(x3, CONST_US_PER_SECOND(st)); /* us */ + PyObject *us_per_sec = get_const_us_per_second(st); + if (us_per_sec == NULL) { + goto Done; + } + x1 = PyNumber_Multiply(x3, us_per_sec); /* us */ + Py_DECREF(us_per_sec); if (x1 == NULL) goto Done; Py_SETREF(x3, NULL); @@ -2225,7 +2198,12 @@ microseconds_to_delta_ex(PyObject *pyus, PyTypeObject *type) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - tuple = checked_divmod(pyus, CONST_US_PER_SECOND(st)); + PyObject *us_per_sec = get_const_us_per_second(st); + if (us_per_sec == NULL) { + goto Done; + } + tuple = checked_divmod(pyus, us_per_sec); + Py_DECREF(us_per_sec); if (tuple == NULL) { goto Done; } @@ -2243,7 +2221,12 @@ microseconds_to_delta_ex(PyObject *pyus, PyTypeObject *type) num = Py_NewRef(PyTuple_GET_ITEM(tuple, 0)); /* leftover seconds */ Py_DECREF(tuple); - tuple = checked_divmod(num, CONST_SEC_PER_DAY(st)); + PyObject *sec_per_day = get_const_sec_per_day(st); + if (sec_per_day == NULL) { + goto Done; + } + tuple = checked_divmod(num, sec_per_day); + Py_DECREF(sec_per_day); if (tuple == NULL) goto Done; Py_DECREF(num); @@ -2849,27 +2832,57 @@ delta_new(PyTypeObject *type, PyObject *args, PyObject *kw) CLEANUP; } if (ms) { - y = accum("milliseconds", x, ms, CONST_US_PER_MS(st), &leftover_us); + PyObject *us_per_ms = get_const_us_per_ms(st); + if (us_per_ms == NULL) { + goto Done; + } + y = accum("milliseconds", x, ms, us_per_ms, &leftover_us); + Py_DECREF(us_per_ms); CLEANUP; } if (second) { - y = accum("seconds", x, second, CONST_US_PER_SECOND(st), &leftover_us); + PyObject *us_per_sec = get_const_us_per_second(st); + if (us_per_sec == NULL) { + goto Done; + } + y = accum("seconds", x, second, us_per_sec, &leftover_us); + Py_DECREF(us_per_sec); CLEANUP; } if (minute) { - y = accum("minutes", x, minute, CONST_US_PER_MINUTE(st), &leftover_us); + PyObject *us_per_min = get_const_us_per_minute(st); + if (us_per_min == NULL) { + goto Done; + } + y = accum("minutes", x, minute, us_per_min, &leftover_us); + Py_DECREF(us_per_min); CLEANUP; } if (hour) { - y = accum("hours", x, hour, CONST_US_PER_HOUR(st), &leftover_us); + PyObject *us_per_hour = get_const_us_per_hour(st); + if (us_per_hour == NULL) { + goto Done; + } + y = accum("hours", x, hour, us_per_hour, &leftover_us); + Py_DECREF(us_per_hour); CLEANUP; } if (day) { - y = accum("days", x, day, CONST_US_PER_DAY(st), &leftover_us); + PyObject *us_per_day = get_const_us_per_day(st); + if (us_per_day == NULL) { + goto Done; + } + y = accum("days", x, day, us_per_day, &leftover_us); + Py_DECREF(us_per_day); CLEANUP; } if (week) { - y = accum("weeks", x, week, CONST_US_PER_WEEK(st), &leftover_us); + PyObject *us_per_week = get_const_us_per_week(st); + if (us_per_week == NULL) { + goto Done; + } + y = accum("weeks", x, week, us_per_week, &leftover_us); + Py_DECREF(us_per_week); CLEANUP; } if (leftover_us) { @@ -3019,7 +3032,7 @@ delta_getstate(PyDateTime_Delta *self) static PyObject * delta_total_seconds(PyObject *op, PyObject *Py_UNUSED(dummy)) { - PyObject *total_seconds; + PyObject *total_seconds = NULL; PyObject *total_microseconds; total_microseconds = delta_to_microseconds(PyDelta_CAST(op)); @@ -3029,8 +3042,13 @@ delta_total_seconds(PyObject *op, PyObject *Py_UNUSED(dummy)) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - total_seconds = PyNumber_TrueDivide(total_microseconds, CONST_US_PER_SECOND(st)); - + PyObject *us_per_sec = get_const_us_per_second(st); + if (us_per_sec == NULL) { + goto finally; + } + total_seconds = PyNumber_TrueDivide(total_microseconds, us_per_sec); + Py_DECREF(us_per_sec); +finally: RELEASE_CURRENT_STATE(st, current_mod); Py_DECREF(total_microseconds); return total_seconds; @@ -6612,7 +6630,12 @@ local_timezone(PyDateTime_DateTime *utc_time) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - delta = datetime_subtract((PyObject *)utc_time, CONST_EPOCH(st)); + PyObject *epoch = get_const_epoch(st); + if (epoch == NULL) { + return NULL; + } + delta = datetime_subtract((PyObject *)utc_time, epoch); + Py_DECREF(epoch); RELEASE_CURRENT_STATE(st, current_mod); if (delta == NULL) return NULL; @@ -6856,8 +6879,12 @@ datetime_timestamp(PyObject *op, PyObject *Py_UNUSED(dummy)) PyObject *current_mod = NULL; datetime_state *st = GET_CURRENT_STATE(current_mod); - PyObject *delta; - delta = datetime_subtract(op, CONST_EPOCH(st)); + PyObject *epoch = get_const_epoch(st); + if (epoch == NULL) { + return NULL; + } + PyObject *delta = datetime_subtract(op, epoch); + Py_DECREF(epoch); RELEASE_CURRENT_STATE(st, current_mod); if (delta == NULL) return NULL; @@ -7224,6 +7251,56 @@ create_timezone_from_delta(int days, int sec, int ms, int normalize) * Module state lifecycle. */ +static inline PyObject * +get_const_us_per_ms(datetime_state *st) { + return st && st->us_per_ms ? Py_NewRef(st->us_per_ms) + : PyLong_FromLong(1000); +} + +static inline PyObject * +get_const_us_per_second(datetime_state *st) { + return st && st->us_per_second ? Py_NewRef(st->us_per_second) + : PyLong_FromLong(1000000); +} + +static inline PyObject * +get_const_us_per_minute(datetime_state *st) { + return st && st->us_per_minute ? Py_NewRef(st->us_per_minute) + : PyLong_FromLong(60000000); +} + +static inline PyObject * +get_const_us_per_hour(datetime_state *st) { + return st && st->us_per_hour ? Py_NewRef(st->us_per_hour) + : PyLong_FromDouble(3600000000.0); +} + +static inline PyObject * +get_const_us_per_day(datetime_state *st) { + return st && st->us_per_day ? Py_NewRef(st->us_per_day) + : PyLong_FromDouble(86400000000.0); +} + +static inline PyObject * +get_const_us_per_week(datetime_state *st) { + return st && st->us_per_week ? Py_NewRef(st->us_per_week) + : PyLong_FromDouble(604800000000.0); +} + +static inline PyObject * +get_const_sec_per_day(datetime_state *st) { + return st && st->seconds_per_day ? Py_NewRef(st->seconds_per_day) + : PyLong_FromLong(24 * 3600); +} + +static inline PyObject * +get_const_epoch(datetime_state *st) { + return st && st->epoch ? Py_NewRef(st->epoch) + : new_datetime(1970, 1, 1, 0, 0, 0, 0, + (PyObject *)&utc_timezone, 0); +} + + static int init_state(datetime_state *st, PyObject *module, PyObject *old_module) { @@ -7243,19 +7320,19 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) return 0; } - st->us_per_ms = CONST_US_PER_MS(NULL); + st->us_per_ms = get_const_us_per_ms(NULL); if (st->us_per_ms == NULL) { return -1; } - st->us_per_second = CONST_US_PER_SECOND(NULL); + st->us_per_second = get_const_us_per_second(NULL); if (st->us_per_second == NULL) { return -1; } - st->us_per_minute = CONST_US_PER_MINUTE(NULL); + st->us_per_minute = get_const_us_per_minute(NULL); if (st->us_per_minute == NULL) { return -1; } - st->seconds_per_day = CONST_SEC_PER_DAY(NULL); + st->seconds_per_day = get_const_sec_per_day(NULL); if (st->seconds_per_day == NULL) { return -1; } @@ -7263,21 +7340,21 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) /* The rest are too big for 32-bit ints, but even * us_per_week fits in 40 bits, so doubles should be exact. */ - st->us_per_hour = CONST_US_PER_HOUR(NULL); + st->us_per_hour = get_const_us_per_hour(NULL); if (st->us_per_hour == NULL) { return -1; } - st->us_per_day = CONST_US_PER_DAY(NULL); + st->us_per_day = get_const_us_per_day(NULL); if (st->us_per_day == NULL) { return -1; } - st->us_per_week = CONST_US_PER_WEEK(NULL); + st->us_per_week = get_const_us_per_week(NULL); if (st->us_per_week == NULL) { return -1; } /* Init Unix epoch */ - st->epoch = CONST_EPOCH(NULL); + st->epoch = get_const_epoch(NULL); if (st->epoch == NULL) { return -1; } From 07ca91fbfdeef59934e95ff416b31a4cbefc9d5a Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Thu, 8 May 2025 14:07:18 +0900 Subject: [PATCH 30/34] Ditto --- Modules/_datetimemodule.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index f0f1cfeaeb1017..b882eb70bc80cc 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -6632,6 +6632,7 @@ local_timezone(PyDateTime_DateTime *utc_time) PyObject *epoch = get_const_epoch(st); if (epoch == NULL) { + RELEASE_CURRENT_STATE(st, current_mod); return NULL; } delta = datetime_subtract((PyObject *)utc_time, epoch); @@ -6881,6 +6882,7 @@ datetime_timestamp(PyObject *op, PyObject *Py_UNUSED(dummy)) PyObject *epoch = get_const_epoch(st); if (epoch == NULL) { + RELEASE_CURRENT_STATE(st, current_mod); return NULL; } PyObject *delta = datetime_subtract(op, epoch); From 13b065f7abe9376bcf05e537596ca02b41169183 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 9 May 2025 09:00:53 +0900 Subject: [PATCH 31/34] Move up a function --- Modules/_datetimemodule.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index b882eb70bc80cc..aed49385208f10 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -7271,6 +7271,15 @@ get_const_us_per_minute(datetime_state *st) { : PyLong_FromLong(60000000); } +static inline PyObject * +get_const_sec_per_day(datetime_state *st) { + return st && st->seconds_per_day ? Py_NewRef(st->seconds_per_day) + : PyLong_FromLong(24 * 3600); +} + +/* The rest are too big for 32-bit ints, but even + * us_per_week fits in 40 bits, so doubles should be exact. + */ static inline PyObject * get_const_us_per_hour(datetime_state *st) { return st && st->us_per_hour ? Py_NewRef(st->us_per_hour) @@ -7289,11 +7298,6 @@ get_const_us_per_week(datetime_state *st) { : PyLong_FromDouble(604800000000.0); } -static inline PyObject * -get_const_sec_per_day(datetime_state *st) { - return st && st->seconds_per_day ? Py_NewRef(st->seconds_per_day) - : PyLong_FromLong(24 * 3600); -} static inline PyObject * get_const_epoch(datetime_state *st) { From 00524513813f7589a8f4689535581e68ec7e78b4 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 13 May 2025 20:24:37 +0900 Subject: [PATCH 32/34] Smaller patch for tests --- Lib/test/datetimetester.py | 3 +-- Lib/test/test_embed.py | 2 +- Modules/_testcapi/datetime.c | 32 -------------------------------- 3 files changed, 2 insertions(+), 35 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 2406a1f7ad13bb..b2025131924bab 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7270,8 +7270,7 @@ def test_update_type_cache(self): assert isinstance(_datetime.timezone.utc, _datetime.tzinfo) del sys.modules['_datetime'] """) - res = script_helper.assert_python_ok('-c', script) - self.assertFalse(res.err) + script_helper.assert_python_ok('-c', script) def test_module_free(self): script = textwrap.dedent(""" diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index 38fb22310aa49f..7bae71a51beb8b 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -461,7 +461,7 @@ def test_datetime_capi_at_shutdown(self): import sys import _testcapi _testcapi.test_datetime_capi() # PyDateTime_IMPORT only once - timedelta = _testcapi.get_capi_types()['timedelta'] + timedelta = type(_testcapi.get_delta_fromdsu(False, 1, 0, 0)) def gen(): try: diff --git a/Modules/_testcapi/datetime.c b/Modules/_testcapi/datetime.c index 36fd9e50ca154c..b800f9b8eb3473 100644 --- a/Modules/_testcapi/datetime.c +++ b/Modules/_testcapi/datetime.c @@ -453,37 +453,6 @@ test_PyDateTime_DELTA_GET(PyObject *self, PyObject *obj) return Py_BuildValue("(iii)", days, seconds, microseconds); } -static PyObject * -get_capi_types(PyObject *self, PyObject *args) -{ - if (PyDateTimeAPI == NULL) { - Py_RETURN_NONE; - } - PyObject *dict = PyDict_New(); - if (dict == NULL) { - return NULL; - } - if (PyDict_SetItemString(dict, "date", (PyObject *)PyDateTimeAPI->DateType) < 0) { - goto error; - } - if (PyDict_SetItemString(dict, "time", (PyObject *)PyDateTimeAPI->TimeType) < 0) { - goto error; - } - if (PyDict_SetItemString(dict, "datetime", (PyObject *)PyDateTimeAPI->DateTimeType) < 0) { - goto error; - } - if (PyDict_SetItemString(dict, "timedelta", (PyObject *)PyDateTimeAPI->DeltaType) < 0) { - goto error; - } - if (PyDict_SetItemString(dict, "tzinfo", (PyObject *)PyDateTimeAPI->TZInfoType) < 0) { - goto error; - } - return dict; -error: - Py_DECREF(dict); - return NULL; -} - static PyMethodDef test_methods[] = { {"PyDateTime_DATE_GET", test_PyDateTime_DATE_GET, METH_O}, {"PyDateTime_DELTA_GET", test_PyDateTime_DELTA_GET, METH_O}, @@ -504,7 +473,6 @@ static PyMethodDef test_methods[] = { {"get_time_fromtimeandfold", get_time_fromtimeandfold, METH_VARARGS}, {"get_timezone_utc_capi", get_timezone_utc_capi, METH_VARARGS}, {"get_timezones_offset_zero", get_timezones_offset_zero, METH_NOARGS}, - {"get_capi_types", get_capi_types, METH_NOARGS}, {"make_timezones_capi", make_timezones_capi, METH_NOARGS}, {"test_datetime_capi", test_datetime_capi, METH_NOARGS}, {NULL}, From d2e2a11c1c0a53f72bad0f37afee36e49195a824 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Mon, 19 May 2025 02:23:27 +0900 Subject: [PATCH 33/34] Add a non-issue test case (closure) --- Lib/test/datetimetester.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index 3b96cd856d853e..f6607d45359aca 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7331,6 +7331,24 @@ def gen(): res = script_helper.assert_python_ok('-X', 'showrefcount', '-c', script) self.assertIn(b'[0 refs, 0 blocks]', res.err) + with self.subTest('With closure'): + # Finalization does not happen when a generator is nested + script = textwrap.dedent(""" + def no_issue(): + def gen(): + try: + yield + finally: + assert sys.modules + import sys + it = gen() + next(it) + + exec(no_issue.__code__) + """) + res = script_helper.assert_python_ok('-c', script) + self.assertFalse(res.err) + def load_tests(loader, standard_tests, pattern): standard_tests.addTest(ZoneInfoCompleteTest()) From 2da198e2a1a525ca70595f88034fb8711592a23d Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Tue, 24 Jun 2025 05:06:56 +0900 Subject: [PATCH 34/34] minimize test --- Lib/test/datetimetester.py | 63 +++----------------------------------- Lib/test/test_embed.py | 42 ------------------------- 2 files changed, 4 insertions(+), 101 deletions(-) diff --git a/Lib/test/datetimetester.py b/Lib/test/datetimetester.py index f6607d45359aca..49e78f6f9a74cd 100644 --- a/Lib/test/datetimetester.py +++ b/Lib/test/datetimetester.py @@ -7275,36 +7275,9 @@ def test_update_type_cache(self): """) script_helper.assert_python_ok('-c', script) - def test_module_free(self): - script = textwrap.dedent(""" - import sys - import gc - import weakref - ws = weakref.WeakSet() - for _ in range(3): - import _datetime - timedelta = _datetime.timedelta # static type - ws.add(_datetime) - del sys.modules['_datetime'] - del _datetime - gc.collect() - assert len(ws) == 0 - """) - script_helper.assert_python_ok('-c', script) - - @unittest.skipIf(not support.Py_DEBUG, "Debug builds only") - def test_no_leak(self): - script = textwrap.dedent(""" - import datetime - datetime.datetime.strptime('20000101', '%Y%m%d').strftime('%Y%m%d') - """) - res = script_helper.assert_python_ok('-X', 'showrefcount', '-c', script) - self.assertIn(b'[0 refs, 0 blocks]', res.err) - def test_static_type_at_shutdown(self): # gh-132413 script = textwrap.dedent(""" - import sys import _datetime timedelta = _datetime.timedelta @@ -7312,42 +7285,14 @@ def gen(): try: yield finally: - # Exceptions are ignored here - assert not sys.modules - td = _datetime.timedelta(days=1) - assert td.days == 1 - td = timedelta(days=1) - assert td.days == 1 - assert not sys.modules + # sys.modules is empty + _datetime.timedelta(days=1) + timedelta(days=1) it = gen() next(it) """) - res = script_helper.assert_python_ok('-c', script) - self.assertFalse(res.err) - - if support.Py_DEBUG: - with self.subTest('Refleak'): - res = script_helper.assert_python_ok('-X', 'showrefcount', '-c', script) - self.assertIn(b'[0 refs, 0 blocks]', res.err) - - with self.subTest('With closure'): - # Finalization does not happen when a generator is nested - script = textwrap.dedent(""" - def no_issue(): - def gen(): - try: - yield - finally: - assert sys.modules - import sys - it = gen() - next(it) - - exec(no_issue.__code__) - """) - res = script_helper.assert_python_ok('-c', script) - self.assertFalse(res.err) + script_helper.assert_python_ok('-c', script) def load_tests(loader, standard_tests, pattern): diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index 61699161c95087..46222e521aead8 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -440,48 +440,6 @@ def test_datetime_reset_strptime(self): out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) self.assertEqual(out, '20000101\n' * INIT_LOOPS) - def test_datetime_capi_type_address(self): - # Check if the C-API types keep their addresses until runtime shutdown - code = textwrap.dedent(""" - import _datetime as d - print( - f'{id(d.date)}' - f'{id(d.time)}' - f'{id(d.datetime)}' - f'{id(d.timedelta)}' - f'{id(d.tzinfo)}' - ) - """) - out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) - self.assertEqual(len(set(out.splitlines())), 1) - - def test_datetime_capi_at_shutdown(self): - # gh-132413 - code = textwrap.dedent(""" - import sys - import _testcapi - _testcapi.test_datetime_capi() # PyDateTime_IMPORT only once - timedelta = type(_testcapi.get_delta_fromdsu(False, 1, 0, 0)) - - def gen(): - try: - yield - finally: - assert not sys.modules - res = 0 - try: - timedelta(days=1) - res = 1 - except ImportError: - res = 2 - print(res) - - it = gen() - next(it) - """) - out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) - self.assertEqual(out, '1\n' * INIT_LOOPS) - def test_static_types_inherited_slots(self): script = textwrap.dedent(""" import test.support