gh-71587: Establish global state in _datetime

[erlend-aasland]

• Establish global state
• Put singletons in global state

• Issue: Datetime NoneType after calling Py_Finalize and Py_Initialize #71587

[vstinner]

Maybe with shorter names, you can just write get_datetime_state()->utc directly (for example).

[erlend-aasland]

PTAL, @vstinner

[vstinner]

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?

[erlend-aasland]

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?

Well, it is easy to already now add the machinery needed to clear the state (future m_clear function). It's easy to add such a function now, and call it in the PyInit__datetime() error path. It will be an improvement over status quo where global state is overwritten anyway.

Note that parts of the state actually is cleared if the module exec function fails.

[erlend-aasland]

Perhaps we should start with a PR that cleans up _datetime init code?

[vstinner]

Apparently, now exec function has two different parts:

• First part uses return -1
• Second part initializes the static state and uses goto to make sure that the state always stays in a consistent state: either fully initialized, or cleared

I suggest to split the function in two parts (create a subfunction). I don't think that a second PR is needed.

Maybe just add a second _datetime_init_state() function?

[erlend-aasland]

Sure, we can bake it into this PR. I'll get to it later tonight.

[erlend-aasland]

I added some helpers in a9e8d1a (init_state and create_timezone_from_delta). Initialisation should now be slightly more robust than before, unless I missed something :)

[erlend-aasland]

If we want to initialise the state members in the order they are declared, we'll have to move this. However, the following comment will then need to be tweaked. I'm not sure it is worth it.

[vstinner]

LGTM, thanks.

[erlend-aasland]

Thanks for the review, Victor!

[erlend-aasland]

There are lots of paths we can take for the follow-up PRs.

Alt 1, continue adding to static state:

• Convert static types to heap types; add them to static state
• Convert static state to module state (will break encapsulated C API)
• Deprecate encapsulated C API and introduce a new one based on module state

Alt 2, try to save the encapsulated C API by moving static state to the interpreter

• Move existing static state to the interpreter
• Convert static types to heap types; add them to the interpreter owned state struct

What do you think, Victor?

[vstinner]

Convert static types to heap types; add them to static state

Sounds like a good step.

Move existing static state to the interpreter

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:

• One state per interpreter: anything involing types and instances of these types
• One state per module instance: constants like "microseconds per second"

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);
}

[vstinner]

    if (state == NULL) {
        return -1;
    }

By the way, the error case could be avoided by running the initialization at Python startup.

[jackyk02]

Hi Erlend, would it be possible for you to support this for Python 3.12?

[erlend-aasland]

Hi Erlend, would it be possible for you to support this for Python 3.12?

No; that would be a breaking change.