pybind11 icon indicating copy to clipboard operation
pybind11 copied to clipboard

[BUG]: pybind11's initialization of internals needs to be made more thread safe in the presence of free threading

Open EthanSteinberg opened this issue 6 months ago • 2 comments

Required prerequisites

  • [X] Make sure you've read the documentation. Your issue may be addressed there.
  • [X] Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
  • [X] Consider asking first in the Gitter chat room or in a Discussion.

What version (or hash if on master) of pybind11 are you using?

a1d00916b26b187e583f3bce39cd59c3b0652c32

Problem description

https://github.com/pybind/pybind11/blob/a1d00916b26b187e583f3bce39cd59c3b0652c32/include/pybind11/detail/internals.h#L498 is not thread safe under free-threading.

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

The problem is 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/a1d00916b26b187e583f3bce39cd59c3b0652c32/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 that is problematic with the new python free threading setup.

In the short term this is still OK because the GIL is re-enabled during module import, but that is not a good long term strategy.

Reproducible example code

No response

Is this a regression? Put the last known working version here if it is.

Not a regression

EthanSteinberg avatar Aug 18 '24 05:08 EthanSteinberg