Skip to content

[6.0][Concurrency] Fix crash from calling fromTaskExecutorPreference after enqueueing job. #76780

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

Conversation

mikeash
Copy link
Contributor

@mikeash mikeash commented Sep 30, 2024

Cherry-pick of #76757 to release/6.0.

After we've enqueued a job, another thread may run it and destroy it. Don't try to get the job's task executor preference when we try to schedule it. Instead, get the task executor preference before enqueueing the job, then use that preference when scheduling if necessary. Since getting the executor preference is potentially somewhat expensive (we need to search the status records for an executor preference record), only do this if the pre-compare-and-swap states look like they'll need it.

rdar://136281920

…after enqueueing job.

After we've enqueued a job, another thread may run it and destroy it. Don't try to get the job's task executor preference when we try to schedule it. Instead, get the task executor preference before enqueueing the job, then use that preference when scheduling if necessary. Since getting the executor preference is potentially somewhat expensive (we need to search the status records for an executor preference record), only do this if the pre-compare-and-swap states look like they'll need it.

rdar://136281920
(cherry picked from commit efa62e7)
@mikeash mikeash requested a review from a team as a code owner September 30, 2024 14:22
@mikeash
Copy link
Contributor Author

mikeash commented Sep 30, 2024

Explanation: When enqueueing a job on an actor, in certain circumstances we then retrieve the job's task executor preference as part of scheduling the work on the actor. However, it's possible for another thread to start draining the actor, run the job, and destroy it in that interval. This change fixes it by retrieving the task executor preference before enqueueing the job.
Scope: Concurrency runtime fix, bug was introduced in 6.0.
Issue: rdar://136281920
Original PR: #76757
Risk: Low. The same work is done, but now we retrieve one piece of data earlier, when the job containing that data is guaranteed to still be live.
Testing: Manual testing to verify that this change fixes the crash. Swift CI testing to verify the rest of Concurrency still works.
Reviewer: @ktoso

@mikeash
Copy link
Contributor Author

mikeash commented Sep 30, 2024

@swift-ci please test

@mikeash mikeash enabled auto-merge September 30, 2024 15:51
@mikeash mikeash merged commit 9bb9688 into swiftlang:release/6.0 Sep 30, 2024
5 checks passed
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.

4 participants