gh-71587: Drop local reference cache to `_strptime` module in `_datetime` by neonene · Pull Request #120224 · python/cpython · GitHub

For 3.13 and newer, this PR fixes the following errors by making each loaded _datetime module use its own reference cache to _strptime module. The cache will be the same as the reference in sys.modules of the corresponding interpreter by importing _strptime module on each strptime() function call.

Main interpreter:

>_testembed_d test_repeated_init_exec "import datetime; datetime.datetime.strptime('200001', '%Y%m')"
--- Loop #1 ---
--- Loop #2 ---
Traceback (most recent call last):
  File "<string>", line 1, in <module>
TypeError: 'NoneType' object is not callable

Sub interpreter:

>>> import _interpreters
>>> for i in range(1,3):
...     print('Sub Loop', i)
...     interp = _interpreters.create()
...     s = "import datetime; datetime.datetime.strptime('200001', '%Y%m')"
...     _interpreters.run_string(interp, s)
...     _interpreters.destroy(interp)
...
Sub Loop 1
Sub Loop 2
Assertion failed: PyUnicode_CheckExact(ep_key), file C:\cp\Objects\dictobject.c, line 1126

cc @ericsnowcurrently @erlend-aasland

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

LGTM

@kumaraditya303 Thanks for the review.

@ericsnowcurrently Maybe off-topic now, but is it possible to access the module state through a static type like:

// pycore_typeobject.h

typedef struct {
    PyTypeObject *type;
    int isbuiltin;
    ....
+   PyObject *current_ext_module  // weak ref? or borrowed ref?
} managed_static_type_state;

Then, get_current_module(&PyDateTime_DateType, &reloading) in _datetimemodule.c?

@ericsnowcurrently Maybe off-topic now, but is it possible to access the module state through a static type like:
// pycore_typeobject.h

typedef struct {
    PyTypeObject *type;
    int isbuiltin;
    ....
+   PyObject *current_ext_module  // weak ref? or borrowed ref?
} managed_static_type_state;
Then, get_current_module(&PyDateTime_DateType, &reloading) in _datetimemodule.c?

That definitely could work. It's worth looking into for 3.14.

LGTM

This should be backported to 3.13, right?

I agree to the 3.13 backport.

Thanks @neonene for the PR, and @ericsnowcurrently for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

…_datetime` (pythongh-120224) The _strptime module object was cached in a static local variable (in the datetime.strptime() implementation). That's a problem when it crosses isolation boundaries, such as reinitializing the runtme or between interpreters. This change fixes the problem by dropping the static variable, instead always relying on the normal sys.modules cache (via PyImport_Import()). (cherry picked from commit 127c1d2) Co-authored-by: neonene <53406459+neonene@users.noreply.github.com>

GH-120424 is a backport of this pull request to the 3.13 branch.

I just realized this fixes a long-standing problem and is not related to the recent work on _datetime. I'm guessing it would make sense to backport this to 3.12. (It's too late for 3.11.)

Thanks @neonene for the PR, and @ericsnowcurrently for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

Sorry, @neonene and @ericsnowcurrently, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 127c1d2771749853e287632c086b6054212bf12a 3.12

@neonene, would you have time to do the 3.12 backport? I'm sure it won't require much effort.

…`_datetime` (gh-120424) The _strptime module object was cached in a static local variable (in the datetime.strptime() implementation). That's a problem when it crosses isolation boundaries, such as reinitializing the runtme or between interpreters. This change fixes the problem by dropping the static variable, instead always relying on the normal sys.modules cache (via PyImport_Import()). (cherry picked from commit 127c1d2, AKA gh-120224) Co-authored-by: neonene <53406459+neonene@users.noreply.github.com>

GH-120431 is a backport of this pull request to the 3.12 branch.

Thanks @ericsnowcurrently, @erlend-aasland for reviewing this, and thanks again @kumaraditya303.

…`_datetime` (gh-120431) The _strptime module object was cached in a static local variable (in the datetime.strptime() implementation). That's a problem when it crosses isolation boundaries, such as reinitializing the runtme or between interpreters. This change fixes the problem by dropping the static variable, instead always relying on the normal sys.modules cache (via PyImport_Import()). (cherry picked from commit 127c1d2, AKA gh-120224)

…_datetime` (pythongh-120224) The _strptime module object was cached in a static local variable (in the datetime.strptime() implementation). That's a problem when it crosses isolation boundaries, such as reinitializing the runtme or between interpreters. This change fixes the problem by dropping the static variable, instead always relying on the normal sys.modules cache (via PyImport_Import()).

neonene added 3 commits June 7, 2024 20:58

add test

2b82d53

make cache per-module

085e37c

add NEWS

e65bec3

neonene requested review from abalkin and pganssle as code owners June 7, 2024 12:21

bedevere-app Bot added the awaiting review label Jun 7, 2024

bedevere-app Bot mentioned this pull request Jun 7, 2024

Datetime NoneType after calling Py_Finalize and Py_Initialize #71587

Closed

neonene added 2 commits June 7, 2024 21:40

fix NEWS

6bb0141

do not copy _strptime from old module state

8a0ea7b

erlend-aasland approved these changes Jun 7, 2024

View reviewed changes

Comment thread Modules/_datetimemodule.c Outdated

Comment thread Modules/_datetimemodule.c Outdated

Comment thread Modules/_datetimemodule.c Outdated

bedevere-app Bot added awaiting merge and removed awaiting review labels Jun 7, 2024

neonene commented Jun 7, 2024

View reviewed changes

Comment thread Modules/_datetimemodule.c Outdated

neonene and others added 2 commits June 7, 2024 22:50

Apply suggestions from code review

39a4259

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>

insert a comment

33e0966

ericsnowcurrently reviewed Jun 7, 2024

View reviewed changes

Comment thread Modules/_datetimemodule.c

Comment thread Modules/_datetimemodule.c Outdated

use PyImport_Import(&_Py_ID(_strptime)) without cache

12fad4c

neonene changed the title gh-71587: Fix reference caching to _strptime module in _datetime gh-71587: Drop local reference cache to _strptime module in _datetime Jun 7, 2024

neonene added 2 commits June 8, 2024 19:49

Update globals-to-fix.tsv

341561b

Merge branch 'main' into strptime

ba2d0a5

kumaraditya303 approved these changes Jun 8, 2024

View reviewed changes

neonene mentioned this pull request Jun 9, 2024

Isolate the _datetime extension module #117398

Closed

ericsnowcurrently approved these changes Jun 12, 2024

View reviewed changes

Comment thread Modules/_datetimemodule.c

ericsnowcurrently added the needs backport to 3.13 bugs and security fixes label Jun 12, 2024

ericsnowcurrently merged commit 127c1d2 into python:main Jun 12, 2024

bedevere-app Bot removed the awaiting merge label Jun 12, 2024

bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Jun 12, 2024

ericsnowcurrently added the needs backport to 3.12 only security fixes label Jun 12, 2024

miss-islington-app Bot assigned ericsnowcurrently Jun 12, 2024

ericsnowcurrently mentioned this pull request Jun 12, 2024

[3.11] gh-117914: fix datetime_strptime when restart cpython #117915

Closed

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Jun 12, 2024

ericsnowcurrently mentioned this pull request Jun 12, 2024

[3.12] gh-71587: Drop local reference cache to _strptime module in _datetime (GH-120224) #120431

Merged

neonene deleted the strptime branch June 12, 2024 20:32

maurycy mentioned this pull request Apr 4, 2026

datetime: cache module lookups (2x faster) #148085

Open

gh-71587: Drop local reference cache to `_strptime` module in `_datetime` by neonene · Pull Request #120224 · python/cpython · GitHub

Uh oh!

Uh oh!

gh-71587: Drop local reference cache to _strptime module in _datetime - #120224

gh-71587: Drop local reference cache to _strptime module in _datetime#120224ericsnowcurrently merged 10 commits intopython:mainpython/cpython:mainfrom neonene:strptimeCopy head branch name to clipboard

Conversation

neonene commented Jun 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kumaraditya303 left a comment

Choose a reason for hiding this comment

Uh oh!

neonene commented Jun 9, 2024

Uh oh!

ericsnowcurrently commented Jun 12, 2024

Uh oh!

ericsnowcurrently left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ericsnowcurrently commented Jun 12, 2024

Uh oh!

neonene commented Jun 12, 2024

Uh oh!

miss-islington-app Bot commented Jun 12, 2024

Uh oh!

bedevere-app Bot commented Jun 12, 2024

Uh oh!

ericsnowcurrently commented Jun 12, 2024

Uh oh!

miss-islington-app Bot commented Jun 12, 2024

Uh oh!

miss-islington-app Bot commented Jun 12, 2024

Uh oh!

ericsnowcurrently commented Jun 12, 2024

Uh oh!

bedevere-app Bot commented Jun 12, 2024

Uh oh!

neonene commented Jun 12, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gh-71587: Drop local reference cache to _strptime module in _datetime#120224
ericsnowcurrently merged 10 commits into
python:mainfrom
neonene:strptime

neonene commented Jun 7, 2024 •
edited
Loading