Skip to content

[5.5][concurrency] Implement support for Task local exclusivity access sets. #38598

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

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Jul 23, 2021

The implemented semantics are that:

  1. Tasks have separate exclusivity access sets.
  2. Any synchronous context that creates tasks will have its exclusive access set merged into the Tasks while the Task is running.

rdar://80492364


CCC

Explanation: Exclusivity and concurrency are incompatible due to exclusivity relying on thread local storage to store exclusive access sets. This results in different Tasks potentially having the same access set (breaking the program model where we have not defined any such relationship) and also the possibility of an access associated with a task beginning on one thread and then ending on a different thread if the task runs on a different thread (causing the exclusivity runtime to crash since it can't find the access).

In this change, I fix these issues by:

  1. Reserving 2 pointers in each AsyncTask to store an access set.
  2. When the task begins executing on a thread, we merge its current access set into the exclusivity thread local access set and when the task ends, we unmerge the Task's access set from the exclusivity access set and serialize the task access set into the task. This is safe since we are guaranteed by the model that all async Tasks will be started from a synchronous context (when we switch tasks, we always pop to a sync context and then push the new context).

Scope: Without this change, Tasks will (depending if they are on the same thread) hit exclusivity violations and the runtime may hit nullptrs at runtime if an access for a Task begins on one thread and is ended on a different Thread that the task runs upon.

SR Issue: rdar://80492364

Risk: Medium. Since this is a semantic change, I think it is a medium risk change. That being said, I think the exhaustive testing significantly reduces the risk.

Testing: I wrote exhaustive tests of all of the cases that the runtime can hit. I provided both runtime tests and specific logging based FileCheck tests.

Reviewer: @mikeash

Example:

// This example crashes without the change since our two Tasks do not have separate exclusivity access sets.
@MainActor func callee2 (_ x: inout Int) async -> Void { ... }

@MainActor
func callee1(_ x: inout Int) async -> () {
    let handle = Task { @MainActor in
        // ==><==. Crash!!! Simultaneous Access?!
        await callee2(&global1)
    }
    await handle.value
}

@MainActor func test() async {
    await callee1(&global1)
}

gottesmm added 2 commits July 22, 2021 17:26
…PRINTF_DEBUG allow it to be used to log Task completion.

This should let people debug Task lifetime easier.

(cherry picked from commit ff6b67c)
…g a job synchronously on an executor.

I am going to use this to test that we propagate synchronous accesses into
asynchronous tasks access sets.

To ensure this is not ABI, I underscored/marked this as alwaysEmitIntoClient.

(cherry picked from commit fc4d6c8)
@gottesmm gottesmm requested review from mikeash and rjmccall July 23, 2021 00:32
@gottesmm gottesmm requested a review from a team as a code owner July 23, 2021 00:32
@gottesmm gottesmm changed the title [concurrency] Implement support for Task local exclusivity access sets. [5.5][concurrency] Implement support for Task local exclusivity access sets. Jul 23, 2021
@gottesmm
Copy link
Contributor Author

@swift-ci test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 0d4bd2ff5e97a1330f8c049e9a67337818520515

@gottesmm gottesmm force-pushed the release/5.5/rdar80492364 branch from 0d4bd2f to e7bed8d Compare July 23, 2021 02:01
@gottesmm
Copy link
Contributor Author

@swift-ci test

gottesmm added 6 commits July 22, 2021 20:05
The implemented semantics are that:

1. Tasks have separate exclusivity access sets.
2. Any synchronous context that creates tasks will have its exclusive access set
   merged into the Tasks while the Task is running.

rdar://80492364
(cherry picked from commit 0aec816)

Conflicts:
	stdlib/public/Concurrency/TaskPrivate.h
…e exclusivity part of the runtime to emit logging information.

The specific environment variable is SWIFT_DEBUG_RUNTIME_EXCLUSIVITY_LOGGING. We only
check the environment the first time we try to lookup the TLSContext, so it
shouldn't impact exclusivity performance.

My intention is to use this to write some FileCheck tests so that we can really
be sure that the runtime is doing what we think it is. As such I also changed
the small amount of logging routines meant for use in the debugger to use stdout.

(cherry picked from commit f79108c)
…builds to more rigorously test the Task based swizzling AccessSet functionality.

I was writing a bunch of functional tests and realized I needed to be more
rigorous and get actual runtime logging to be able to be sure that the runtime
feature was working correctly.

(cherry picked from commit 7a41f4c)
…m to hang.

Custom executor isn't a true feature yet, so I am disabling on Windows until we
can look at why the hangs are occuring on Windows.

(cherry picked from commit dae605a)
@gottesmm gottesmm force-pushed the release/5.5/rdar80492364 branch from e7bed8d to cdac1ea Compare July 23, 2021 03:05
@gottesmm
Copy link
Contributor Author

@swift-ci test

@gottesmm
Copy link
Contributor Author

@swift-ci nominate

@gottesmm gottesmm merged commit 158e170 into swiftlang:release/5.5 Jul 23, 2021
@gottesmm gottesmm deleted the release/5.5/rdar80492364 branch July 23, 2021 19:19
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