Skip to content
New issue

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

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

Already on GitHub? # to your account

gh-110415: move datetime global state into module state #110420

Closed

Conversation

costasgambit
Copy link

@costasgambit costasgambit commented Oct 5, 2023

datetime has types and various singletons defined as static global variables. _datetime_exec() has an initialisation step that writes into those structs on module import.

If the module is imported by the main interpreter and a sub-interpreter, or by two sub-interpreters, then the process crashes.

This is a first draft MR that:

  • Moves the global bits of state into a DateTimeState struct / _PyModule_GetState
  • Changes module initialisation to use PyModuleDef_Init with Py_MOD_PER_INTERPRETER_GIL_SUPPORTED
  • Changes the type definitions to use the more modern slots style

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Oct 5, 2023

The following commit authors need to sign the Contributor License Agreement:

Click the button to sign:
CLA not signed

@bedevere-app
Copy link

bedevere-app bot commented Oct 5, 2023

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@bedevere-app
Copy link

bedevere-app bot commented Oct 6, 2023

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@bedevere-app
Copy link

bedevere-app bot commented Oct 6, 2023

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@bedevere-app
Copy link

bedevere-app bot commented Oct 6, 2023

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@bedevere-app
Copy link

bedevere-app bot commented Oct 6, 2023

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@erlend-aasland erlend-aasland self-requested a review October 6, 2023 09:04
@erlend-aasland
Copy link
Contributor

See previous work:

@erlend-aasland
Copy link
Contributor

./python -m test -R : test_datetime results in 39 failures and 7 errors; please investigate.

@costasgambit
Copy link
Author

I'm closing this, since it's not finished yet and is a duplicate.

The test failures in question are due to a global (and rather bad-smelling) capsule object that I hadn't gotten round to yet.

If dealing with that would involve a C api change then some process will be required.

However, arguably with a bit of effort the capsule could be made to work if used from the main interpreter only and throw if one attempts to re-import it from elsewhere. That way that particular C api will stay the same for legacy extensions that rely on it, but the datetime module itself can become sub-interpreter compatible.

@erlend-aasland
Copy link
Contributor

Yeah, the encapsulated C API is a big pain. One (or possibly the only) possibility is to put the module state into the interpreter state. No matter how it is solved, it will break all users of the datetime C API (which is quite a few). There is no quick or easy fix :(

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants