Skip to content

[5.5] Inject hop_to_executor after self is fully initialized in unrestricted actor inits #38490

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 7 commits into from
Jul 20, 2021

Conversation

kavon
Copy link
Member

@kavon kavon commented Jul 19, 2021

Rationale: The typechecker permits unfettered use of self throughout an actor's async initializer. This patch brings the implementation of such initializers in line with what the typechecker permits, by actually hopping to self's initializer in order to reserve it. This prevents a possible race with the initializer's body.
Risk: Low
Risk Detail: This change is focused on correcting the code generated by the compiler.
Reward: High
Reward Details: Fixes a miscompile that would lead to a concurrency bug.
Original PR: #38215
Issue: rdar://78790683
Code Reviewed By: Konrad Malawski
Testing Details: Regression test is included.

kavon added 6 commits July 19, 2021 16:25
I ran into this while implementing
async actor inits; where I was emitting
an actor hop in the middle of
an access region, which caused the
access set to be unexpectedly empty.
Because `self` can be used unrestricted after it is
fully-initialized in an async actor init, we perform
a hop_to_executor(self) immediately after `self`
is fully-initialized on all paths within the
initializer.

If we did not, then code could race with the
initializer if it, e.g., spawns a task from
the initializer and mutates actor state that
the initializer then reads. By hopping to the
executor, we prevent that task from running
until _after_ the initializer completes.
@kavon kavon requested a review from a team as a code owner July 19, 2021 23:35
@kavon
Copy link
Member Author

kavon commented Jul 19, 2021

@swift-ci please test

@kavon kavon added bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. concurrency Feature: umbrella label for concurrency language features r5.5 labels Jul 19, 2021
@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 83b54f8

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 83b54f8

@kavon
Copy link
Member Author

kavon commented Jul 20, 2021

@swift-ci please test

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.

Read through and looks solid.

@gottesmm and @atrick also reviewed it previously in the original PR.

@kavon
Copy link
Member Author

kavon commented Jul 20, 2021

@swift-ci please nominate

@DougGregor DougGregor merged commit 22dd46f into swiftlang:release/5.5 Jul 20, 2021
@kavon kavon deleted the 5.5-flowsensitive-init-hop branch July 20, 2021 18:49
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.5 labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. concurrency Feature: umbrella label for concurrency language features 🍒 release cherry pick Flag: Release branch cherry picks swift 5.5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants