Skip to content

datetime: Stop Exposing Process-Global Objects in the datetime C-API #122184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ericsnowcurrently opened this issue Jul 23, 2024 · 3 comments
Open
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Jul 23, 2024

Feature or enhancement

Proposal:

The datetime module has its own C-API which is enabled with PyDateTime_IMPORT. From the docs:

Before using any of these functions, the header file datetime.h must be included in your source
(note that this is not included by Python.h), and the macro PyDateTime_IMPORT must be invoked,
usually as part of the module initialisation function. The macro puts a pointer to a C structure into
a static variable, PyDateTimeAPI, that is used by the following macros.

My main concern is that the PyDateTimeAPI struct is a process-global value, but it exposes object pointers (which should always be per-interpreter). We have worked around this in 3.13+, but it would be better if we could make the objects per-interpreter.

FTR, here are the objects exposed directly by PyDateTimeAPI:

  • (static type) PyDateTime_DateType
  • (static type) PyDateTime_DateTimeType
  • (static type) PyDateTime_TimeType
  • (static type) PyDateTime_DeltaType
  • (static type) PyDateTime_TZInfoType
  • (singleton) utc_timezone (an instance of PyDateTime_TimeZoneType)

exposed indirectly:

  • (static type) PyDateTime_TimeZoneType

In order to make these objects per-interpreter, it would require changes to the datetime C-API. 1 I expect we would leave PyDateTime_IMPORT alone. Instead, we'd need to update the macros in datetime.h to get the objects from the module associated with the current interpreter. 2

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Footnotes

  1. These would be ABI-incompatible changes, which wouldn't be a problem unless the datetime C-API is part of the limited API).

  2. Anyone who is accessing the PyDateTimeAPI struct directly would have to change their code. It might make sense to provide a getter function/macro for each of the objects.

@ericsnowcurrently ericsnowcurrently added type-feature A feature request or enhancement extension-modules C modules in the Modules dir 3.14 bugs and security fixes labels Jul 23, 2024
@ngoldbaum
Copy link
Contributor

ngoldbaum commented Oct 15, 2024

I was looking at the PyDateTime wrappers in PyO3 today and noticed a tiny weirdness. PyDateTime_IMPORT is implemented using PyCapsule_Import, which can in principle fail, but the macro doesn't do any error checking:

cpython/Include/datetime.h

Lines 199 to 200 in 12eaadc

#define PyDateTime_IMPORT \
PyDateTimeAPI = (PyDateTime_CAPI *)PyCapsule_Import(PyDateTime_CAPSULE_NAME, 0)

In practice I guess this means the macro (and implicitly that use of PyCapsule_Import) can't ever fail. It would be nice to document that if it really can't fail, or I guess fix this issue if in principle the PyCapsule_Import could fail.

@davidhewitt
Copy link
Contributor

I believe that it definitely can fail, e.g. if a user names a file datetime.py in the current directory then that can win import resolution, after which point the capsule import fails. At least, I had a historical crash in PyO3 which I had to deal with this case: PyO3/pyo3#3818

@ngoldbaum
Copy link
Contributor

I made a small C reproducer to illustrate the problem and created capi-workgroup/problems#81 so this issue can go back to its original purpose.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension-modules C modules in the Modules dir type-feature A feature request or enhancement
Projects
Development

No branches or pull requests

4 participants