Skip to content

[Concurrency] Remove _unsafeInheritExecutor from public APIs, use #isolation #72578

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 3 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 43 additions & 8 deletions stdlib/public/Concurrency/CheckedContinuation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -281,19 +281,37 @@ extension CheckedContinuation {
/// - SeeAlso: `withCheckedThrowingContinuation(function:_:)`
/// - SeeAlso: `withUnsafeContinuation(function:_:)`
/// - SeeAlso: `withUnsafeThrowingContinuation(function:_:)`
@available(SwiftStdlib 5.1, *)
@_unsafeInheritExecutor // ABI compatibility with Swift 5.1
@inlinable
@_unavailableInEmbedded
@available(SwiftStdlib 5.1, *)
#if !$Embedded
@backDeployed(before: SwiftStdlib 6.0)
#endif
public func withCheckedContinuation<T>(
function: String = #function,
_ body: (CheckedContinuation<T, Never>) -> Void
isolation: isolated (any Actor)? = #isolation,
function: String = #function,
_ body: (CheckedContinuation<T, Never>) -> Void
) async -> T {
return await withUnsafeContinuation {
body(CheckedContinuation(continuation: $0, function: function))
}
}

@available(SwiftStdlib 5.1, *)
@usableFromInline
@_unsafeInheritExecutor // ABI compatibility with Swift 5.1
@_unavailableInEmbedded
@_silgen_name("$ss23withCheckedContinuation8function_xSS_yScCyxs5NeverOGXEtYalF")
internal func __abi_withCheckedContinuation<T>(
function: String = #function,
_ body: (CheckedContinuation<T, Never>) -> Void
) async -> T {
return await withUnsafeContinuation {
body(CheckedContinuation(continuation: $0, function: function))
}
}


/// Invokes the passed in closure with a checked continuation for the current task.
///
/// The body of the closure executes synchronously on the calling task, and once it returns
Expand Down Expand Up @@ -322,13 +340,30 @@ public func withCheckedContinuation<T>(
/// - SeeAlso: `withCheckedContinuation(function:_:)`
/// - SeeAlso: `withUnsafeContinuation(function:_:)`
/// - SeeAlso: `withUnsafeThrowingContinuation(function:_:)`
@available(SwiftStdlib 5.1, *)
@_unsafeInheritExecutor // ABI compatibility with Swift 5.1
@inlinable
@_unavailableInEmbedded
@available(SwiftStdlib 5.1, *)
#if !$Embedded
@backDeployed(before: SwiftStdlib 6.0)
#endif
public func withCheckedThrowingContinuation<T>(
function: String = #function,
_ body: (CheckedContinuation<T, Error>) -> Void
isolation: isolated (any Actor)? = #isolation,
function: String = #function,
_ body: (CheckedContinuation<T, Error>) -> Void
) async throws -> T {
return try await withUnsafeThrowingContinuation {
body(CheckedContinuation(continuation: $0, function: function))
}
}

@available(SwiftStdlib 5.1, *)
@usableFromInline
@_unsafeInheritExecutor // ABI compatibility with Swift 5.1
@_unavailableInEmbedded
@_silgen_name("$ss31withCheckedThrowingContinuation8function_xSS_yScCyxs5Error_pGXEtYaKlF")
internal func __abi_withCheckedThrowingContinuation<T>(
function: String = #function,
_ body: (CheckedContinuation<T, Error>) -> Void
) async throws -> T {
return try await withUnsafeThrowingContinuation {
body(CheckedContinuation(continuation: $0, function: function))
Expand Down
4 changes: 2 additions & 2 deletions stdlib/public/Concurrency/PartialAsyncTask.swift
Original file line number Diff line number Diff line change
Expand Up @@ -598,9 +598,9 @@ internal func _resumeUnsafeThrowingContinuationWithError<T>(
/// - SeeAlso: `withCheckedContinuation(function:_:)`
/// - SeeAlso: `withCheckedThrowingContinuation(function:_:)`
@available(SwiftStdlib 5.1, *)
@_unsafeInheritExecutor
@_alwaysEmitIntoClient
public func withUnsafeContinuation<T>(
isolation: isolated (any Actor)? = #isolation,
_ fn: (UnsafeContinuation<T, Never>) -> Void
) async -> T {
return await Builtin.withUnsafeContinuation {
Expand Down Expand Up @@ -634,9 +634,9 @@ public func withUnsafeContinuation<T>(
/// - SeeAlso: `withCheckedContinuation(function:_:)`
/// - SeeAlso: `withCheckedThrowingContinuation(function:_:)`
@available(SwiftStdlib 5.1, *)
@_unsafeInheritExecutor
@_alwaysEmitIntoClient
public func withUnsafeThrowingContinuation<T>(
isolation: isolated (any Actor)? = #isolation,
_ fn: (UnsafeContinuation<T, Error>) -> Void
) async throws -> T {
return try await Builtin.withUnsafeThrowingContinuation {
Expand Down
4 changes: 2 additions & 2 deletions stdlib/public/Concurrency/Task.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -346,9 +346,9 @@ static SerialExecutorRef executorForEnqueuedJob(Job *job) {
return SerialExecutorRef::generic();
#else
void *jobQueue = job->SchedulerPrivate[Job::DispatchQueueIndex];
if (jobQueue == DISPATCH_QUEUE_GLOBAL_EXECUTOR)
if (jobQueue == DISPATCH_QUEUE_GLOBAL_EXECUTOR) {
return SerialExecutorRef::generic();
else
} else
return SerialExecutorRef::forOrdinary(reinterpret_cast<HeapObject*>(jobQueue),
_swift_task_getDispatchQueueSerialExecutorWitnessTable());
#endif
Expand Down
45 changes: 41 additions & 4 deletions stdlib/public/Concurrency/TaskLocal.swift
Original file line number Diff line number Diff line change
Expand Up @@ -181,10 +181,27 @@ public final class TaskLocal<Value: Sendable>: Sendable, CustomStringConvertible
/// the operation closure.
@inlinable
@discardableResult
@_unsafeInheritExecutor
@backDeployed(before: SwiftStdlib 5.8)
public func withValue<R>(_ valueDuringOperation: Value, operation: () async throws -> R,
@available(SwiftStdlib 5.1, *)
@backDeployed(before: SwiftStdlib 6.0)
public func withValue<R>(_ valueDuringOperation: Value,
operation: () async throws -> R,
isolation: isolated (any Actor)? = #isolation,
file: String = #fileID, line: UInt = #line) async rethrows -> R {
return try await withValueImpl(
valueDuringOperation,
operation: operation,
isolation: isolation,
file: file, line: line)
}

@usableFromInline
@discardableResult
@_unsafeInheritExecutor // ABI compatibility with Swift 5.1
@available(SwiftStdlib 5.1, *)
@_silgen_name("$ss9TaskLocalC9withValue_9operation4file4lineqd__x_qd__yYaKXESSSutYaKlF")
internal func __abi_withValue<R>(_ valueDuringOperation: Value,
operation: () async throws -> R,
file: String = #fileID, line: UInt = #line) async rethrows -> R {
return try await withValueImpl(valueDuringOperation, operation: operation, file: file, line: line)
}

Expand All @@ -206,11 +223,30 @@ public final class TaskLocal<Value: Sendable>: Sendable, CustomStringConvertible
/// to swift_task_de/alloc for the copy as follows:
/// - withValue contains the compiler-emitted calls swift_task_de/alloc.
/// - withValueImpl contains the calls to _taskLocalValuePush/Pop
@inlinable
@discardableResult
@available(SwiftStdlib 5.1, *)
@backDeployed(before: SwiftStdlib 6.0)
internal func withValueImpl<R>(_ valueDuringOperation: __owned Value,
operation: () async throws -> R,
isolation: isolated (any Actor)?,
file: String = #fileID, line: UInt = #line) async rethrows -> R {
// check if we're not trying to bind a value from an illegal context; this may crash
_checkIllegalTaskLocalBindingWithinWithTaskGroup(file: file, line: line)

_taskLocalValuePush(key: key, value: consume valueDuringOperation)
defer { _taskLocalValuePop() }

return try await operation()
}

@inlinable
@discardableResult
@_unsafeInheritExecutor
@available(SwiftStdlib 5.1, *)
@backDeployed(before: SwiftStdlib 5.9)
internal func withValueImpl<R>(_ valueDuringOperation: __owned Value, operation: () async throws -> R,
internal func withValueImpl<R>(_ valueDuringOperation: __owned Value,
operation: () async throws -> R,
file: String = #fileID, line: UInt = #line) async rethrows -> R {
// check if we're not trying to bind a value from an illegal context; this may crash
_checkIllegalTaskLocalBindingWithinWithTaskGroup(file: file, line: line)
Expand All @@ -221,6 +257,7 @@ public final class TaskLocal<Value: Sendable>: Sendable, CustomStringConvertible
return try await operation()
}


/// Binds the task-local to the specific value for the duration of the
/// synchronous operation.
///
Expand Down
2 changes: 1 addition & 1 deletion stdlib/toolchain/Compatibility56/Concurrency/Actor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class ExecutorTrackingInfo {

/// Unconditionally initialize a fresh tracking state on the
/// current state, shadowing any previous tracking state.
/// leave() must be called beforet the object goes out of scope.
/// leave() must be called before the object goes out of scope.
void enterAndShadow(ExecutorRef currentExecutor) {
ActiveExecutor = currentExecutor;
SavedInfo = ActiveInfoInThread.get();
Expand Down
61 changes: 61 additions & 0 deletions test/Concurrency/Runtime/continuation_validation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,72 @@

import StdlibUnittest

@MainActor
@available(SwiftStdlib 5.1, *)
func test_isolation_withUnsafeContinuation() async {
// This test specifically should have only one suspension point,
// as it would trigger a problem with the previous @_unsafeInheritExecutor
// implementation, where we optimize away a switch accidentally, causing
// wrong isolation.
await withUnsafeContinuation { continuation in
MainActor.shared.assertIsolated() // OK
continuation.resume(returning: ())
}
}
@MainActor
@available(SwiftStdlib 5.1, *)
func test_isolation_withUnsafeThrowingContinuation() async {
// See comment in `test_isolation_withUnsafeContinuation` about exact test case shape
try! await withUnsafeThrowingContinuation { continuation in
MainActor.shared.assertIsolated() // OK
continuation.resume(returning: ())
}
}
@MainActor
@available(SwiftStdlib 5.1, *)
func test_isolation_withCheckedContinuation() async {
// See comment in `test_isolation_withUnsafeContinuation` about exact test case shape
await withCheckedContinuation { continuation in
MainActor.shared.assertIsolated() // OK
continuation.resume(returning: ())
}
}
@MainActor
@available(SwiftStdlib 5.1, *)
func test_isolation_withCheckedThrowingContinuation() async {
// See comment in `test_isolation_withUnsafeContinuation` about exact test case shape
try! await withCheckedThrowingContinuation { continuation in
MainActor.shared.assertIsolated() // OK
continuation.resume(returning: ())
}
}

@main struct Main {
static func main() async {
let tests = TestSuite("ContinuationValidation")

if #available(SwiftStdlib 5.1, *) {
tests.test("withUnsafeThrowingContinuation: continuation should be on calling isolation") {
await Task.detached {
await test_isolation_withUnsafeThrowingContinuation()
}.value
}
tests.test("withUnsafeContinuation: continuation should be on calling isolation") {
await Task.detached {
await test_isolation_withUnsafeContinuation()
}.value
}
tests.test("withCheckedContinuation: continuation should be on calling isolation") {
await Task.detached {
await test_isolation_withCheckedContinuation()
}.value
}
tests.test("withCheckedThrowingContinuation: continuation should be on calling isolation") {
await Task.detached {
await test_isolation_withCheckedThrowingContinuation()
}.value
}

tests.test("trap on double resume of unchecked continuation") {
expectCrashLater(withMessage: "may have already been resumed")

Expand Down
5 changes: 2 additions & 3 deletions test/Concurrency/async_task_locals_basic_warnings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,18 @@
// REQUIRES: concurrency
// REQUIRES: asserts

@available(SwiftStdlib 5.1, *)
actor Test {

@TaskLocal static var local: Int?

func run() async {
// This should NOT produce any warnings, the closure withValue uses is @Sendable:
await Test.$local.withValue(42) {
await Self.$local.withValue(42) {
await work()
}
}

func work() async {
print("Hello \(Test.local ?? 0)")
print("Hello \(Self.local ?? 0)")
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/OtherActors.swiftmodule -module-name OtherActors %S/Inputs/OtherActors.swift -disable-availability-checking

// RUN: %target-swift-frontend -I %t -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify
// RUN: %target-swift-frontend -I %t -disable-availability-checking -strict-concurrency=complete -parse-as-library %s -emit-sil -o /dev/null -verify -enable-upcoming-feature RegionBasedIsolation

// REQUIRES: concurrency
// REQUIRES: asserts

// FIXME: rdar://125078448 is resolved
// XFAIL: *

actor Test {

@TaskLocal static var local: Int?

func withTaskLocal(isolation: isolated (any Actor)? = #isolation,
_ body: (consuming NonSendableValue, isolated (any Actor)?) -> Void) async {
Self.$local.withValue(12) {
// Unexpected errors here:
// error: unexpected warning produced: transferring 'body' may cause a race; this is an error in the Swift 6 language mode
// error: unexpected note produced: actor-isolated 'body' is captured by a actor-isolated closure. actor-isolated uses in closure may race against later nonisolated uses
body(NonSendableValue(), isolation)
}
}
}

class NonSendableValue {}
16 changes: 16 additions & 0 deletions test/abi/macOS/arm64/concurrency.swift
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,14 @@ Added: _swift_task_getPreferredTaskExecutor
Added: _swift_task_popTaskExecutorPreference
Added: _swift_task_pushTaskExecutorPreference

// Adopt #isolation in with...Continuation APIs
// Swift.withCheckedThrowingContinuation<A>(isolation: isolated Swift.Actor?, function: Swift.String, _: (Swift.CheckedContinuation<A, Swift.Error>) -> ()) async throws -> A
Added: _$ss31withCheckedThrowingContinuation9isolation8function_xScA_pSgYi_SSyScCyxs5Error_pGXEtYaKlF
Added: _$ss31withCheckedThrowingContinuation9isolation8function_xScA_pSgYi_SSyScCyxs5Error_pGXEtYaKlFTu
// Swift.withCheckedContinuation<A>(isolation: isolated Swift.Actor?, function: Swift.String, _: (Swift.CheckedContinuation<A, Swift.Never>) -> ()) async -> A
Added: _$ss23withCheckedContinuation9isolation8function_xScA_pSgYi_SSyScCyxs5NeverOGXEtYalF
Added: _$ss23withCheckedContinuation9isolation8function_xScA_pSgYi_SSyScCyxs5NeverOGXEtYalFTu

// Updated signature for withTaskExecutorPreference to used typed throws and #isolation
// Swift.withTaskExecutorPreference<A, B where B: Swift.Error>(_: Swift.TaskExecutor?, isolation: isolated Swift.Actor?, operation: () async throws(B) -> A) async throws(B) -> A
Added: _$ss26withTaskExecutorPreference_9isolation9operationxSch_pSg_ScA_pSgYixyYaq_YKXEtYaq_YKs5ErrorR_r0_lF
Expand Down Expand Up @@ -303,3 +311,11 @@ Added: _$sScfsE13checkIsolatedyyF
Added: _$sScf13checkIsolatedyyFTj
// method descriptor for Swift.SerialExecutor.checkIsolated() -> ()
Added: _$sScf13checkIsolatedyyFTq

// #isolated adoption in TaskLocal.withValue
// Swift.TaskLocal.withValueImpl<A>(_: __owned A, operation: () async throws -> A1, isolation: isolated Swift.Actor?, file: Swift.String, line: Swift.UInt) async throws -> A1
Added: _$ss9TaskLocalC13withValueImpl_9operation9isolation4file4lineqd__xn_qd__yYaKXEScA_pSgYiSSSutYaKlF
Added: _$ss9TaskLocalC13withValueImpl_9operation9isolation4file4lineqd__xn_qd__yYaKXEScA_pSgYiSSSutYaKlFTu
// Swift.TaskLocal.withValue<A>(_: A, operation: () async throws -> A1, isolation: isolated Swift.Actor?, file: Swift.String, line: Swift.UInt) async throws -> A1
Added: _$ss9TaskLocalC9withValue_9operation9isolation4file4lineqd__x_qd__yYaKXEScA_pSgYiSSSutYaKlF
Added: _$ss9TaskLocalC9withValue_9operation9isolation4file4lineqd__x_qd__yYaKXEScA_pSgYiSSSutYaKlFTu
16 changes: 16 additions & 0 deletions test/abi/macOS/x86_64/concurrency.swift
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,14 @@ Added: _swift_task_getPreferredTaskExecutor
Added: _swift_task_popTaskExecutorPreference
Added: _swift_task_pushTaskExecutorPreference

// Adopt #isolation in with...Continuation APIs
// Swift.withCheckedThrowingContinuation<A>(isolation: isolated Swift.Actor?, function: Swift.String, _: (Swift.CheckedContinuation<A, Swift.Error>) -> ()) async throws -> A
Added: _$ss31withCheckedThrowingContinuation9isolation8function_xScA_pSgYi_SSyScCyxs5Error_pGXEtYaKlF
Added: _$ss31withCheckedThrowingContinuation9isolation8function_xScA_pSgYi_SSyScCyxs5Error_pGXEtYaKlFTu
// Swift.withCheckedContinuation<A>(isolation: isolated Swift.Actor?, function: Swift.String, _: (Swift.CheckedContinuation<A, Swift.Never>) -> ()) async -> A
Added: _$ss23withCheckedContinuation9isolation8function_xScA_pSgYi_SSyScCyxs5NeverOGXEtYalF
Added: _$ss23withCheckedContinuation9isolation8function_xScA_pSgYi_SSyScCyxs5NeverOGXEtYalFTu

// Updated signature for withTaskExecutorPreference to used typed throws and #isolation
// Swift.withTaskExecutorPreference<A, B where B: Swift.Error>(_: Swift.TaskExecutor?, isolation: isolated Swift.Actor?, operation: () async throws(B) -> A) async throws(B) -> A
Added: _$ss26withTaskExecutorPreference_9isolation9operationxSch_pSg_ScA_pSgYixyYaq_YKXEtYaq_YKs5ErrorR_r0_lF
Expand Down Expand Up @@ -303,3 +311,11 @@ Added: _$sScfsE13checkIsolatedyyF
Added: _$sScf13checkIsolatedyyFTj
// method descriptor for Swift.SerialExecutor.checkIsolated() -> ()
Added: _$sScf13checkIsolatedyyFTq

// #isolated adoption in TaskLocal.withValue
// Swift.TaskLocal.withValueImpl<A>(_: __owned A, operation: () async throws -> A1, isolation: isolated Swift.Actor?, file: Swift.String, line: Swift.UInt) async throws -> A1
Added: _$ss9TaskLocalC13withValueImpl_9operation9isolation4file4lineqd__xn_qd__yYaKXEScA_pSgYiSSSutYaKlF
Added: _$ss9TaskLocalC13withValueImpl_9operation9isolation4file4lineqd__xn_qd__yYaKXEScA_pSgYiSSSutYaKlFTu
// Swift.TaskLocal.withValue<A>(_: A, operation: () async throws -> A1, isolation: isolated Swift.Actor?, file: Swift.String, line: Swift.UInt) async throws -> A1
Added: _$ss9TaskLocalC9withValue_9operation9isolation4file4lineqd__x_qd__yYaKXEScA_pSgYiSSSutYaKlF
Added: _$ss9TaskLocalC9withValue_9operation9isolation4file4lineqd__x_qd__yYaKXEScA_pSgYiSSSutYaKlFTu
Loading