Skip to content

[BUG]: gil_safe_call_once_and_store is not NoGil thread safe. #5245

New issue

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

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

Already on GitHub? Sign in to your account

Closed
3 tasks done
albanD opened this issue Jul 15, 2024 · 1 comment · Fixed by #5246
Closed
3 tasks done

[BUG]: gil_safe_call_once_and_store is not NoGil thread safe. #5245

albanD opened this issue Jul 15, 2024 · 1 comment · Fixed by #5246
Labels
triage New bug, unverified

Comments

@albanD
Copy link
Contributor

albanD commented Jul 15, 2024

Required prerequisites

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

0e2c3e5

Problem description

In the function,

if (!is_initialized_) { // This read is guarded by the GIL.
// Multiple threads may enter here, because the GIL is released in the next line and
// CPython API calls in the `fn()` call below may release and reacquire the GIL.
gil_scoped_release gil_rel; // Needed to establish lock ordering.
std::call_once(once_flag_, [&] {
// Only one thread will ever enter here.
gil_scoped_acquire gil_acq;
::new (storage_) T(fn()); // fn may release, but will reacquire, the GIL.
is_initialized_ = true; // This write is guarded by the GIL.

is_initialized_ is read outside of the critical section without any synchronization. Which can lead to a race in a NoGil build.

Reproducible example code

N/A

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

Not a regression

@albanD albanD added the triage New bug, unverified label Jul 15, 2024
colesbury added a commit to colesbury/pybind11 that referenced this issue Jul 16, 2024
The "is_initialized_" flags is not protected by the GIL in free-threaded
Python, so it needs to be an atomic field.

Fixes pybind#5245
rwgk pushed a commit that referenced this issue Jul 16, 2024
)

* fix: Make gil_safe_call_once thread-safe in free-threaded CPython

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

Fixes #5245

* style: pre-commit fixes

* Apply changes from code review

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@albanD
Copy link
Contributor Author

albanD commented Jul 16, 2024

Thanks for the quick fix !

henryiii pushed a commit that referenced this issue Aug 13, 2024
)

* fix: Make gil_safe_call_once thread-safe in free-threaded CPython

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

Fixes #5245

* style: pre-commit fixes

* Apply changes from code review

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage New bug, unverified
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant