fix: make gil_safe_call_once thread-safe in free-threaded CPython

[colesbury]

Description

The "is_initialized_" flags is not protected by the GIL in free-threaded Python, so it needs to be an atomic field.

Fixes #5245

Suggested changelog entry:

Make ``gil_safe_call_once_and_store`` thread-safe in free-threaded CPython.

[rwgk]

Slight preference for an empty line before the #ifdef.

[rwgk]

Hi @colesbury, considering that Python 3.13.0rc1 is out, we (me, @henryiii, @EthanSteinberg ) are thinking it may be useful to make another pybind11 release, with this PR included. We are wondering: Is there anything else obvious (from your vantage point) that we could/should do in pybind11, before making a release?

[colesbury]

No, I can't think of anything else

[EthanSteinberg]

@colesbury I'm a bit worried about the logic in https://github.com/pybind/pybind11/blob/master/include/pybind11/detail/internals.h#L498 under free-threading and I was hoping I could get your help to double check that everything is thread safe.

internals is a global singleton struct shared across all pybind11 modules. get_internals() either retrieves that current global singleton or creates a new one.

I'm a bit worried about double initialization, where two copies of internals would be created by multiple modules being imported/initalized in different threads.

https://github.com/pybind/pybind11/blob/master/include/pybind11/detail/internals.h#L520-L524 are the key problematic lines where we retrieve the current global, and initialize it if necessary.

The problem is that if two threads hit those lines at the same time, both threads could see that internals is null and both threads could initialize internals, which would lead to two copies.

Currently this is protected via the GIL (https://github.com/pybind/pybind11/blob/master/include/pybind11/detail/internals.h#L505), but does that work with the new free threading code?

[colesbury]

I think it would be better if we made the initialization in get_internals() explicitly thread-safe, but it's unlikely to cause problems because the free-threaded build temporarily enables the GIL when importing C extensions.

Do you want to create an issue for this?

[henryiii]

Another question (based on a question from @EthanSteinberg in separate communication): would it make sense for gil_scoped_acquire to take our lock? Code placed in a gil_scoped_acquire block expects to have a lock.

[colesbury]

No, I think gil_scoped_acquire should just call PyGILState_Ensure or equivalent. If extensions want a global lock, they can skip setting py::mod_gil_not_used.

[EthanSteinberg]

@colesbury Thanks for confirming that the GIL is enabled during module initialization, so this is probably not a concern.

I don't know if it makes sense to make an issue now for this. Instead, let me write a PR next weekend that just adds some comments to the code pointing out why the module initialization GIL behavior allows that code to be safe even with free threading.

[colesbury]

I'd rather not rely on the GIL behavior during module initialization. Additionally, at least in theory, I think get_internals() could be called for the first time after the C API extension was initialized.

[rwgk]

I'd rather not rely on the GIL behavior during module initialization. Additionally, at least in theory, I think get_internals() could be called for the first time after the C API extension was initialized.

I'm not sure if it's helpful, but JIC, get_internals() is called from here (PYBIND11_MODULE() macro):

• pybind11/include/pybind11/detail/common.h

  Line 489 in fc59f4e

  PYBIND11_ENSURE_INTERNALS_READY \

[EthanSteinberg]

@colesbury @rwgk I created an issue to track / solve this #5316