Isolate the _datetime extension module

[neonene]

Feature or enhancement

Proposal:

I hope this issue will complete _datetime isolation.

My concerns (and answers)

• Py_MOD_PER_INTERPRETER_GIL_SUPPORTED should be applied in sync with _zoninfo?

  • YES: Possible.

• Can a module state have a C-API structure, keeping a capsule just for comatibility?

  • NO: Possible, but a user should not touch the module state.

• C-API supports only the main interpreter? Otherwise, PyInterpreterState is acceptable to point each structure?

  • NO: PyDateTimeAPI cannot emit an error. Also, no PyInterpreterState member is accessible from datetime.h. UPDATE: Seems to be possible by using a global function pointer instead of a function.

Specific issue:

• Initialize/Finalize Python multiple time and import datetime each time lead to a memory corruption #118608

Links to previous discussion of this feature:

• Isolate Stdlib Extension Modules #103092
• gh-71587: Isolate _datetime #102995
• gh-71587: Establish global state in _datetime #110475

Linked PRs (closed)

• gh-117398: datetime: Support per-interpreter C-API #117399
• gh-117398: datetime: Make use of C-API capsules for sub-interpreters #117413
• gh-117398: datetime: Use wrappers for C-API functions #118115
• gh-117398: datetime: Access C-API via PyInterpreterState #118357

Linked PRs

• gh-117398: Isolate _datetime (3.13.0a6 PoC) #117498
• gh-117398: Isolate _datetime (3.13.0a6 PoC) with AC #118337
• gh-117398: Move types to datetime state #118606
• gh-117398: Add multiphase support to _datetime #119373
• gh-117398: Statically Allocate the Datetime C-API #119472
• gh-117398: Add datetime C-API type check test for subinterpreters #119604
• [3.13] gh-117398: Add multiphase support to _datetime (gh-119373) #119636
• gh-117398: Convert datetime.IsoCalendarDate To A Heap Type #119637
• [3.13] gh-117398: Revert gh-119636, Add multiphase support to _datetime #119639
• gh-117398: Add datetime Module State #119640
• [3.13] gh-117398: Statically Allocate the Datetime C-API (GH-119472) #119641
• gh-117398: gh-119655: datetime: Init static state once & don't free it #119662
• [3.13] gh-117398: Add multiphase support to _datetime #119694
• [3.13] gh-117398: Convert datetime.IsoCalendarDate To A Heap Type (gh-119637) #119695
• gh-117398: Add datetime Module State #119810
• gh-117398: Use Per-Interpreter State for the _datetime Static Types #119929
• [3.13] gh-117398: Add datetime Module State (gh-119810) #120004
• [3.13] gh-117398: Use Per-Interpreter State for the _datetime Static Types (gh-119929) #120009
• gh-71587: Drop local reference cache to _strptime module in _datetime #120224
• [3.13] gh-117398: Add datetime C-API type check test for subinterpreters (gh-119604) #120463
• gh-117398: Use the correct module loader for iOS in datetime CAPI test. #120477
• [3.13] gh-117398: Use the correct module loader for iOS in datetime CAPI test (GH-120477) #121561

[neonene]

PR #117399 fails except Windows. Is there an easy way to access a PyInterpreterState member? I'll try later allowing C-API only for the main-interpreter with a global variable.

[erlend-aasland]

I briefly discussed the C API capsule with @pganssle on the 2023 (or was it 2022?) language summit. I also asked the Steering Council to comment about backwards compatibility concerns regarding datetime.h (which is not included by Python.h):

• Request for comment: datetime.h and PEP-387 steering-council#208

Putting it in the PyInterpreterState struct is an option, but I'm not sure how much we want to bloat that struct. We probably want to deprecate the current capsulated API and introduce a new capsulated API.

[vstinner]

Converting the _datetime extension to multiphase initialization and/or converting static types to heap types is complicated because:

• <Include/datetime.h> C API
• datetime capsule (C API)
• If the _datetime extension is reloaded, the C API expects to meet the same types. Otherwise, PyDateTime_Check() fails and everything else fails.

It reminds me the very complicated case of the PyAST C API. We managed to convert the _ast extension to heap types and multiphase init by moving state to the interpreter state. Other "trade offs" attemps for the _ast extension ended by introducing crashes which were hard to trigger and hard to fix.

So I propose this plan to isolate the datetime module:

• Move references to types to datetime_state structure: I wrote a minimum PR gh-117398: Move types to datetime state #118606 for that
• Convert static types to heap types, but create them only once, and don't delete them (on purpose, for now).
• Move datetime_state to PyInterpreterState.
• At the point, discuss how to deal with remaining issues: multiphase init, C API, capsule, etc.

[neonene]

Where can we see your rationale for the inflating PyInterpreterState with PyAST C-API? IIUC, the practice is supposed to be bad: #103092 (comment).

How do you associate the PyAST issue with PyDateTime in terms of the mechanizm and impact?

@encukou, @ericsnowcurrently

[vstinner]

How do you associate the PyAST issue with PyDateTime in terms of the mechanizm and impact?

Both provide a C API and we wanted to isolate their C extension.

Where can we see your rationale for the inflating PyInterpreterState with PyAST C-API?

I will try to dig into the bug tracker later. In short, there were 2-3 crashes related to the isolation of the _ast extension. Crashes related to the C API.

[neonene]

Python-ast.c (3.10.0 alpha 0: b1cc6ba)

int PyAST_Check(PyObject* obj)
{
    astmodulestate *state = get_global_ast_state();
    if (state == NULL) {
        return -1;
    }
    return PyObject_IsInstance(obj, state->AST_type);
}

Previously, PyAST_Check() imported the _ast module through get_global_ast_state(). The implicit import could get an unsafe pseudo module by replacing __import__() with a lazy import (strongly discouraged since Python 3.3).

• https://bugzilla.redhat.com/show_bug.cgi?id=1871992#c1
• bpo-41631: _ast module uses again a global state #21961
• bpo-41796: Make _ast module state per interpreter #23024

datetime.h

#define PyDate_Check(op) PyObject_TypeCheck((op), PyDateTimeAPI->DateType)
#define PyDate_CheckExact(op) Py_IS_TYPE((op), PyDateTimeAPI->DateType)

PyDate_Check() requires the real module to be imported in advance by calling PyCapsule_Import() explicitly.

At least, the regression case in PyAST C-API above cannot be applied to PyDateTime C-API, I think.

[vstinner]

The purpose of this issue is to convert the _datetime extension to the multiphase C API. The problem is to make sure that we are dealing with the same types if multiple instances of the extension are created.

Example:

import sys

import _datetime
now = _datetime.datetime.now()
del _datetime
del sys.modules['_datetime']

import _datetime
print(isinstance(now, _datetime.datetime))

In general, we don't require this. For example, it's false with the _random extension:

import sys

import _random
rng = _random.Random()
del _random
del sys.modules['_random']

import _random
print(isinstance(rng, _random.Random))

The problem for _datetime is the C API and the capsule. Currently, the C API is based on PyDateTimeAPI variable (one variable is created every time the datetime.h header is included.) I would prefer PyDateTime_Check() to return true even if there are two instances of the datetime module.

I would care less if the C API would be stateful: pass explicitly a module/state/something. But changing the C API is out of the scope of this issue.

[erlend-aasland]

But changing the C API is out of the scope of this issue.

The SC said that the datetime C API is protected by PEP-387, even though datetime.h is not explicitly included in Python.h. IMO, we should deprecate the current C API and introduce a new one. That is the easiest solution technically, and the easiest to relate to for users. I agree with Victor that it deserves its own issue (and perhaps also a Discourse topic).

[vstinner]

I merged my first change "Move types to datetime state".

@neonene: Do you want to propose changes to convert static types to heap types? Maybe start with a PR to convert only 4 types to make the PR short enough so it's easier to review.

Types:

    PyTypeObject *date_type;
    PyTypeObject *datetime_type;
    PyTypeObject *delta_type;
    PyTypeObject *isocalendar_date_type;
    PyTypeObject *time_type;
    PyTypeObject *tzinfo_type;
    PyTypeObject *timezone_type;

[vstinner]

IMO, we should deprecate the current C API and introduce a new one.

If we manage to isolate the _datetime extension without breaking the C API, I'm not sure that it's needed.

[neonene]

I'll wait python/steering-council#243, which is a necessary procedure.

[ncoghlan]

A key functional test case in relation to python/steering-council#243 is ensuring that previous references to the capsule API still work after a module reload. Specifically, this sequence:

1. capsule reference is acquired
2. module is reloaded
3. capsule reference is used

This will have historically worked due to the implicit module caching in the single-phase init support for extension modules, but I'm concerned that a full migration to true reload support will break it (and likely crash the interpreter in the process).

I'd also be genuinely surprised if such a test case already exists, hence the green CI on #118337 may not be enough to show that the migration to multi-phase init hasn't broken anything.