Skip to content

Fix main executor check in startOnMainActor #65271

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 2 commits into from
Apr 19, 2023

Conversation

etcwilde
Copy link
Member

Fix the main executor check in swift_task_startOnMainActorImpl.
Some API's don't set the current executor. In that case, the current executor is a nullptr and the direct check fails.
Using swift_task_isCurrentExecutor checks that we're actually on the main executor in those cases by checking the current dispatch queue identity, and checking that we're on the main thread.

Also adding @discardableResult since we can discard the results from this 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 requested a review from DougGregor April 18, 2023 23:45
@etcwilde
Copy link
Member Author

@swift-ci please test

@etcwilde etcwilde added the concurrency Feature: umbrella label for concurrency language features label Apr 18, 2023
Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@etcwilde etcwilde merged commit b8396e6 into swiftlang:main Apr 19, 2023
@etcwilde etcwilde deleted the ewilde/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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants