Skip to content

[🍒 5.9] Fix main executor check in startOnMainActor #65272

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

Conversation

etcwilde
Copy link
Member

@etcwilde etcwilde commented Apr 18, 2023

Cherry-picking #65271

Explanation: Some API's don't set the current executor resulting in the current executor containing a nullptr identity. This caused the MainExecutor check in swift_task_startOnMainActorImpl to fail, when we were running on the main thread and executing off of the main dispatch queue. swift_task_isCurrentExecutor contains additional logic to properly handle this case, checking that we're running off of the main dispatch queue and that we're running on the main thread. Also adding @discardableResult since we can discard the results from this SPI, like with Task.detached.

Scope: The swift_task_startOnMainActorImpl is only used by the Task.startOnMainActor SPI, so this change only impacts the one SPI function. It's replacing a piece of logic with a function that has been in the runtime for two years now, and is in use in other places in the concurrency runtime for this purpose.

Risk: Low. The SPI was added earlier this release so changing this shouldn't break existing code. Additionally, it's SPI.

rdar://108221645
rdar://108222007

startOnMainActor checked the current executor directly, which in some
cases came back as a `nullptr`. This happens with older API which don't
know to set the current executor. `swift_task_isCurrentExecutor` knows
how to check for the current dispatch queue and the main thread though,
so switching the main-executor check to use that API instead.
Adding discardableResult to startOnMainActor
@etcwilde etcwilde added concurrency Feature: umbrella label for concurrency language features 🍒 release cherry pick Flag: Release branch cherry picks swift 5.9 labels Apr 18, 2023
@etcwilde etcwilde requested a review from a team as a code owner April 18, 2023 23:51
@etcwilde
Copy link
Member Author

@swift-ci please test

@DougGregor
Copy link
Member

@swift-ci please test macOS

@DougGregor DougGregor merged commit 960345e into swiftlang:release/5.9 Apr 19, 2023
@etcwilde etcwilde deleted the ewilde/5.9/fix-startOnMainActor branch April 19, 2023 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
concurrency Feature: umbrella label for concurrency language features 🍒 release cherry pick Flag: Release branch cherry picks swift 5.9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants