Repository navigation
Isolate the _datetime extension module #117398
Description
Activity
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):
Putting it in the
PyInterpreterStatestruct 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.Reacted by Kirill Podoprigora and neoneneConverting 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.
Where can we see your rationale for the inflating
PyInterpreterStatewith 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?
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.
- added a commit that references this issue
on May 8, 2024 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_astmodule throughget_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 callingPyCapsule_Import()explicitly.At least, the regression case in PyAST C-API above cannot be applied to PyDateTime C-API, I think.
- added a commit that references this issue
on May 10, 2024 62 remaining items
- added a commit that references this issue
on Jul 10, 2024 - added 5 commits that reference this issue
on Jul 11, 2024 - added 8 commits that reference this issue
on Jul 17, 2024
Feature or enhancement
Proposal:
I hope this issue will complete
_datetimeisolation.My concerns (and answers)
Py_MOD_PER_INTERPRETER_GIL_SUPPORTEDshould be applied in sync with_zoninfo?Can a module state have a C-API structure, keeping a capsule just for comatibility?
C-API supports only the main interpreter? Otherwise,
PyInterpreterStateis acceptable to point each structure?PyDateTimeAPIcannot emit an error. Also, noPyInterpreterStatemember is accessible fromdatetime.h. UPDATE: Seems to be possible by using a global function pointer instead of a function.Specific issue:
Links to previous discussion of this feature:
_datetime#102995_datetime#110475Linked PRs (closed)
Details
PyInterpreterState#118357Linked PRs
_strptimemodule in_datetime#120224