Skip to content

[5.9] [Concurrency runtime] Don't read from the actor after transitioning state #66009

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 19, 2023
Merged

[5.9] [Concurrency runtime] Don't read from the actor after transitioning state #66009

merged 1 commit into from
May 19, 2023

Conversation

@DougGregor
Copy link
Member

@DougGregor DougGregor commented May 18, 2023 •
edited
Loading

  • Explanation: In the concurrency runtime, calls to the tracing hooks for tracking actor state transitions assumed that they could read from the actor's memory. While unlocking an actor, there is an extremely narrow window in which the actor has transitioned into an idle state and then been deallocated by another thread before the tracing hook gets invoked, causing a use-after-free in in the concurrency runtime. Predetermine the information that was being read from the actor at pointers where we know that the actor is still alive, and pass it through to these tracing functions, to eliminate the use-after-free.
  • Scope: Narrow; moves a load of actor state from one central place (that might be invalid) to a few places where it is guaranteed to be valid.
  • Risk: Low; there's no semantic change here, just moving loads around to safer places in the concurrency runtime.
  • Reviewers: @ktoso, @rjmccall
  • Issue: rdar://108497870
  • Original pull request: [Concurrency runtime] Don't read from the actor after transitioning state #66008

@DougGregor
[Concurrency runtime] Don't read from the actor after transitioning s…
b14e47f 
…tate

Once we have transitioned the actor into a new state, we report the
state change as a trace event so it can be noted by tools (e.g.,
Instruments). However, the act of transitioning to a new state can mean
that there is an opportunity for another thread to deallocate the
actor. This means that the tracing call cannot depend on dereferencing
the actor pointer.

A refactoring a few months ago to move the bit that indicates when a
distributed actor is remote from inside the atomic actor state out to a
separate field (because it's constant for a given actor instance),
which introduced a dereference of the actor instance in forming the
tracing call. This introduced a narrow window in which a race
condition could occur: the actor transitions to an idle state, and is
then deallocate before the trace event for the actor transition occurs,
leading to a use-after-free.

Fetch this bit of information earlier in the process, before any state
changes and when we know the actor is still allocated, and pass it
through to the tracing code.

Fixes rdar://108497870.
@DougGregor DougGregor requested a review from a team as a code owner May 18, 2023 23:13
@DougGregor DougGregor changed the title [Concurrency runtime] Don't read from the actor after transitioning state [5.9] [Concurrency runtime] Don't read from the actor after transitioning state May 18, 2023
@DougGregor
Copy link
Member Author

DougGregor commented May 18, 2023

@swift-ci please test

@DougGregor DougGregor merged commit 3173bc4 into swiftlang:release/5.9 May 19, 2023
@DougGregor DougGregor deleted the actor-state-tracing-race-5.9 branch May 19, 2023 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ktoso ktoso ktoso approved these changes

@nkcsgexi nkcsgexi nkcsgexi approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DougGregor @ktoso @nkcsgexi