From 2b82d533a1bcd6ebfa655e09f02c64082c50e91b Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 7 Jun 2024 20:58:09 +0900 Subject: [PATCH 1/9] add test --- Lib/test/test_embed.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Lib/test/test_embed.py b/Lib/test/test_embed.py index d94c63a13b8ea4..634513ec7a5812 100644 --- a/Lib/test/test_embed.py +++ b/Lib/test/test_embed.py @@ -404,6 +404,15 @@ def test_ucnhash_capi_reset(self): out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) self.assertEqual(out, '9\n' * INIT_LOOPS) + def test_datetime_reset_strptime(self): + code = ( + "import datetime;" + "d = datetime.datetime.strptime('2000-01-01', '%Y-%m-%d');" + "print(d.strftime('%Y%m%d'))" + ) + out, err = self.run_embedded_interpreter("test_repeated_init_exec", code) + self.assertEqual(out, '20000101\n' * INIT_LOOPS) + @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") class InitConfigTests(EmbeddingTestsMixin, unittest.TestCase): From 085e37c9c419607e0c655d7afba9240db40fa271 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 7 Jun 2024 20:58:59 +0900 Subject: [PATCH 2/9] make cache per-module --- Modules/_datetimemodule.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index bea6e9411a75ed..7c6e1e24738882 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -50,6 +50,9 @@ typedef struct { /* The interned Unix epoch datetime instance */ PyObject *epoch; + + /* _strptime module */ + PyObject *strptime; } datetime_state; /* The module has a fixed number of static objects, due to being exposed @@ -5514,19 +5517,27 @@ datetime_utcfromtimestamp(PyObject *cls, PyObject *args) static PyObject * datetime_strptime(PyObject *cls, PyObject *args) { - static PyObject *module = NULL; PyObject *string, *format; if (!PyArg_ParseTuple(args, "UU:strptime", &string, &format)) return NULL; + PyObject *result = NULL; + PyObject *current_mod = NULL; + datetime_state *st = GET_CURRENT_STATE(current_mod); + PyObject *module = st->strptime; if (module == NULL) { module = PyImport_ImportModule("_strptime"); - if (module == NULL) - return NULL; + if (module == NULL) { + goto Done; + } + st->strptime = module; } - return PyObject_CallMethodObjArgs(module, &_Py_ID(_strptime_datetime), + result = PyObject_CallMethodObjArgs(module, &_Py_ID(_strptime_datetime), cls, string, format, NULL); +Done: + RELEASE_CURRENT_STATE(st, current_mod); + return result; } /* Return new datetime from date/datetime and time arguments. */ @@ -7057,6 +7068,7 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) .us_per_week = Py_NewRef(st_old->us_per_week), .seconds_per_day = Py_NewRef(st_old->seconds_per_day), .epoch = Py_NewRef(st_old->epoch), + .strptime = Py_NewRef(st_old->strptime), }; return 0; } @@ -7109,6 +7121,7 @@ traverse_state(datetime_state *st, visitproc visit, void *arg) { /* heap types */ Py_VISIT(st->isocalendar_date_type); + Py_VISIT(st->strptime); return 0; } @@ -7125,6 +7138,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->strptime); return 0; } From e65bec37c111cc1cd55288d114025e9aac063ee5 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 7 Jun 2024 21:00:23 +0900 Subject: [PATCH 3/9] add NEWS --- .../next/Library/2024-06-07-11-23-31.gh-issue-71587.IjFajE.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2024-06-07-11-23-31.gh-issue-71587.IjFajE.rst diff --git a/Misc/NEWS.d/next/Library/2024-06-07-11-23-31.gh-issue-71587.IjFajE.rst b/Misc/NEWS.d/next/Library/2024-06-07-11-23-31.gh-issue-71587.IjFajE.rst new file mode 100644 index 00000000000000..1521dedfb94e16 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-06-07-11-23-31.gh-issue-71587.IjFajE.rst @@ -0,0 +1 @@ +Fix crash in :meth:`_datetime.datetime.strptime` when called again on the restarted interpreter. From 6bb01410e4dd3f83393348fa6627f319587be92d Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 7 Jun 2024 21:40:01 +0900 Subject: [PATCH 4/9] fix NEWS --- .../next/Library/2024-06-07-11-23-31.gh-issue-71587.IjFajE.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-06-07-11-23-31.gh-issue-71587.IjFajE.rst b/Misc/NEWS.d/next/Library/2024-06-07-11-23-31.gh-issue-71587.IjFajE.rst index 1521dedfb94e16..50a662977993f5 100644 --- a/Misc/NEWS.d/next/Library/2024-06-07-11-23-31.gh-issue-71587.IjFajE.rst +++ b/Misc/NEWS.d/next/Library/2024-06-07-11-23-31.gh-issue-71587.IjFajE.rst @@ -1 +1,2 @@ -Fix crash in :meth:`_datetime.datetime.strptime` when called again on the restarted interpreter. +Fix crash in C version of :meth:`datetime.datetime.strptime` when called again +on the restarted interpreter. From 8a0ea7beeb18d6aa8f115b6665ff6ce8c8799a37 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 7 Jun 2024 22:03:10 +0900 Subject: [PATCH 5/9] do not copy _strptime from old module state --- Modules/_datetimemodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 7c6e1e24738882..dba8d83d77bdb7 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -7068,7 +7068,7 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) .us_per_week = Py_NewRef(st_old->us_per_week), .seconds_per_day = Py_NewRef(st_old->seconds_per_day), .epoch = Py_NewRef(st_old->epoch), - .strptime = Py_NewRef(st_old->strptime), + .strptime = st->strptime, }; return 0; } From 39a425984133c852a9e12afed08c44cd6e2a5d35 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 7 Jun 2024 22:50:34 +0900 Subject: [PATCH 6/9] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Modules/_datetimemodule.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index dba8d83d77bdb7..2b81c4eb553120 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -5529,13 +5529,13 @@ datetime_strptime(PyObject *cls, PyObject *args) if (module == NULL) { module = PyImport_ImportModule("_strptime"); if (module == NULL) { - goto Done; + goto exit; } st->strptime = module; } result = PyObject_CallMethodObjArgs(module, &_Py_ID(_strptime_datetime), - cls, string, format, NULL); -Done: + cls, string, format, NULL); +exit: RELEASE_CURRENT_STATE(st, current_mod); return result; } From 33e0966855c2653fc62e20980a812fd1e64bcf76 Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Fri, 7 Jun 2024 23:08:55 +0900 Subject: [PATCH 7/9] insert a comment --- Modules/_datetimemodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index 2b81c4eb553120..b538a6496626ad 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -7121,8 +7121,8 @@ traverse_state(datetime_state *st, visitproc visit, void *arg) { /* heap types */ Py_VISIT(st->isocalendar_date_type); + /* containers */ Py_VISIT(st->strptime); - return 0; } From 12fad4c4d09908f52f1c1a98344bcd9d55580f9f Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sat, 8 Jun 2024 07:09:56 +0900 Subject: [PATCH 8/9] use PyImport_Import(&_Py_ID(_strptime)) without cache --- .../pycore_global_objects_fini_generated.h | 1 + Include/internal/pycore_global_strings.h | 1 + .../internal/pycore_runtime_init_generated.h | 1 + .../internal/pycore_unicodeobject_generated.h | 3 +++ Modules/_datetimemodule.c | 24 ++++--------------- 5 files changed, 11 insertions(+), 19 deletions(-) diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h index b186408931c92e..2ed7e558a5e32d 100644 --- a/Include/internal/pycore_global_objects_fini_generated.h +++ b/Include/internal/pycore_global_objects_fini_generated.h @@ -777,6 +777,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_showwarnmsg)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_shutdown)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_slotnames)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_strptime)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_strptime_datetime)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_swappedbytes_)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_type_)); diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index e1808c85acfb2d..effb3123488d72 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -266,6 +266,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(_showwarnmsg) STRUCT_FOR_ID(_shutdown) STRUCT_FOR_ID(_slotnames) + STRUCT_FOR_ID(_strptime) STRUCT_FOR_ID(_strptime_datetime) STRUCT_FOR_ID(_swappedbytes_) STRUCT_FOR_ID(_type_) diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index 2dde6febc2cae4..960289737608e8 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -775,6 +775,7 @@ extern "C" { INIT_ID(_showwarnmsg), \ INIT_ID(_shutdown), \ INIT_ID(_slotnames), \ + INIT_ID(_strptime), \ INIT_ID(_strptime_datetime), \ INIT_ID(_swappedbytes_), \ INIT_ID(_type_), \ diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h index b00119a1bad7ff..72a8501d87e0f6 100644 --- a/Include/internal/pycore_unicodeobject_generated.h +++ b/Include/internal/pycore_unicodeobject_generated.h @@ -639,6 +639,9 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { string = &_Py_ID(_slotnames); assert(_PyUnicode_CheckConsistency(string, 1)); _PyUnicode_InternInPlace(interp, &string); + string = &_Py_ID(_strptime); + assert(_PyUnicode_CheckConsistency(string, 1)); + _PyUnicode_InternInPlace(interp, &string); string = &_Py_ID(_strptime_datetime); assert(_PyUnicode_CheckConsistency(string, 1)); _PyUnicode_InternInPlace(interp, &string); diff --git a/Modules/_datetimemodule.c b/Modules/_datetimemodule.c index b538a6496626ad..cb4622893375d7 100644 --- a/Modules/_datetimemodule.c +++ b/Modules/_datetimemodule.c @@ -50,9 +50,6 @@ typedef struct { /* The interned Unix epoch datetime instance */ PyObject *epoch; - - /* _strptime module */ - PyObject *strptime; } datetime_state; /* The module has a fixed number of static objects, due to being exposed @@ -5517,26 +5514,18 @@ datetime_utcfromtimestamp(PyObject *cls, PyObject *args) static PyObject * datetime_strptime(PyObject *cls, PyObject *args) { - PyObject *string, *format; + PyObject *string, *format, *result; if (!PyArg_ParseTuple(args, "UU:strptime", &string, &format)) return NULL; - PyObject *result = NULL; - PyObject *current_mod = NULL; - datetime_state *st = GET_CURRENT_STATE(current_mod); - PyObject *module = st->strptime; + PyObject *module = PyImport_Import(&_Py_ID(_strptime)); if (module == NULL) { - module = PyImport_ImportModule("_strptime"); - if (module == NULL) { - goto exit; - } - st->strptime = module; + return NULL; } result = PyObject_CallMethodObjArgs(module, &_Py_ID(_strptime_datetime), cls, string, format, NULL); -exit: - RELEASE_CURRENT_STATE(st, current_mod); + Py_DECREF(module); return result; } @@ -7068,7 +7057,6 @@ init_state(datetime_state *st, PyObject *module, PyObject *old_module) .us_per_week = Py_NewRef(st_old->us_per_week), .seconds_per_day = Py_NewRef(st_old->seconds_per_day), .epoch = Py_NewRef(st_old->epoch), - .strptime = st->strptime, }; return 0; } @@ -7121,8 +7109,7 @@ traverse_state(datetime_state *st, visitproc visit, void *arg) { /* heap types */ Py_VISIT(st->isocalendar_date_type); - /* containers */ - Py_VISIT(st->strptime); + return 0; } @@ -7138,7 +7125,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->strptime); return 0; } From 341561b1ea7a300757895b987867726de5346a2e Mon Sep 17 00:00:00 2001 From: neonene <53406459+neonene@users.noreply.github.com> Date: Sat, 8 Jun 2024 19:49:43 +0900 Subject: [PATCH 9/9] Update globals-to-fix.tsv --- Tools/c-analyzer/cpython/globals-to-fix.tsv | 1 - 1 file changed, 1 deletion(-) diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index 4586a59f6ac2ef..cb9750a69a632b 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -393,7 +393,6 @@ Modules/xxmodule.c - ErrorObject - ## initialized once Modules/_cursesmodule.c - ModDict - -Modules/_datetimemodule.c datetime_strptime module - ## state Modules/_datetimemodule.c - _datetime_global_state -