gh-71587: Establish global state in _datetime#110475
Conversation
vstinner
left a comment
There was a problem hiding this comment.
If PyInit__datetime() fails, I suppose that the next call may override global state without clearing the previous strong refererence: reference leak. I don't think that we should bother since this state is going to become more dynamic and set differently, no?
Sorry, something went wrong.
Well, it is easy to already now add the machinery needed to clear the state (future Note that parts of the state actually is cleared if the module exec function fails. |
Sorry, something went wrong.
|
Perhaps we should start with a PR that cleans up |
Sorry, something went wrong.
vstinner
left a comment
There was a problem hiding this comment.
LGTM, thanks.
Sorry, something went wrong.
|
Thanks for the review, Victor! |
Sorry, something went wrong.
|
There are lots of paths we can take for the follow-up PRs. Alt 1, continue adding to static state:
Alt 2, try to save the encapsulated C API by moving static state to the interpreter
What do you think, Victor? |
Sorry, something went wrong.
Sounds like a good step.
I would prefer to consider that after converting types to heap types. IMO the C API like PyDateTime_Check() will prevent to make this state per module. Instead, you can consider splitting this state in two states:
You simply cannot have more than one instance of the same type beause of PyDateTime_Check(). PyDateTime_Check() will be per-interpreter. I had the exact same problem with PyAST_Check(): int PyAST_Check(PyObject* obj)
{
struct ast_state *state = get_ast_state();
if (state == NULL) {
return -1;
}
return PyObject_IsInstance(obj, state->AST_type);
} |
Sorry, something went wrong.
if (state == NULL) {
return -1;
}By the way, the error case could be avoided by running the initialization at Python startup. |
Sorry, something went wrong.
|
Hi Erlend, would it be possible for you to support this for Python 3.12? |
Sorry, something went wrong.
No; that would be a breaking change. |
Sorry, something went wrong.
* Use explicit initialiser for m_base * Add module state stub; establish global state on stack * Put conversion factors in state struct * Move PyDateTime_TimeZone_UTC to state * Move PyDateTime_Epoch to state struct * Fix ref leaks in and clean up initialisation
edited
LoadingUh oh!
There was an error while loading. Please reload this page.
Copy link Copy MarkdownSorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.