Skip to content
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

Fix potential deadlocks when resuming a continuation while holding a lock #303

Merged
merged 5 commits into from
Nov 17, 2023

Conversation

FranzBusch
Copy link
Member

We wrongly assumed that it is safe to resume continuations while holding our state machine lock. There is a potential for a deadlock in rare circumstances. The problem is that the runtime holds a task specific lock. This lock is both acquired by task cancellation and continuation resumption. This can lead to scenarios where we are resuming the task's continuation while holding the state machine lock and a different thread cancelling the task and holding the runtime lock. Now both locks are held by different threads and they are waiting to acquire the opposite lock; hence, deadlocking the process.

In this PR, I went through all of our state machine based implementations and made sure that we are only acquiring the lock for as little as possible and handle all of the actions outside of the lock.

@ole
Copy link
Contributor

ole commented Nov 2, 2023

Out of curiosity, is this about the issue described in Swift #67110: waitForStatusRecordUnlockIfNotSelfLocked deadlocks on macOS 14.0?

@FranzBusch
Copy link
Member Author

@ole Yes this would fix the issue.

@FranzBusch
Copy link
Member Author

@swift-ci please test

@ole
Copy link
Contributor

ole commented Nov 2, 2023

@FranzBusch Thanks!

@FranzBusch FranzBusch merged commit da4e36f into apple:main Nov 17, 2023
2 checks passed
@FranzBusch FranzBusch deleted the fb-fix-deadlocks branch November 17, 2023 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants