Skip to content

[Threading] Fix C11 once implementation to not rely on lock recursion. #70767

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

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented Jan 8, 2024

The implementation mistakenly locked and unlocked the mutex in helper.once_wait(), which is only ever called from inside a region where the mutex is already locked.

rdar://119739594

The implementation mistakenly locked and unlocked the mutex in
`helper.once_wait()`, which is only ever called from inside a region
where the mutex is already locked.

rdar://119739594
@al45tair al45tair added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.10 labels Jan 8, 2024
@al45tair al45tair requested a review from a team as a code owner January 8, 2024 18:09
@al45tair
Copy link
Contributor Author

al45tair commented Jan 8, 2024

Explanation: The C11 support was developed originally with a shim implementation that wouldn't have picked this problem up (we were accidentally locking and unlocking the mutex twice for the swift::once slow path). Apparently this problem wasn't noticed until now.
Risk: Low. Fixes an obvious bug.
Original PR: #70761
Reviewed by: @mikeash
Resolves: rdar://119739594
Tests: There are already unit tests for this code, but the exact code paths that get tested are platform specific and to exercise this particular bug you'd need a platform with C11 threads that also errors when recursively locking a non-recursive mutex. (CI doesn't have such a thing.)

@al45tair
Copy link
Contributor Author

al45tair commented Jan 8, 2024

@swift-ci Please test

@al45tair al45tair merged commit 5134c21 into swiftlang:release/5.10 Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 5.10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants