From be2b2ad3a4ff64ec6263445655f3ae1c730d91e7 Mon Sep 17 00:00:00 2001 From: Konrad 'ktoso' Malawski Date: Fri, 11 Apr 2025 11:47:09 +0900 Subject: [PATCH 1/2] [Concurrency] Use caller execution for withTaskCannelation handler The problem is that even with the #isolation parameter the non-Sendable async closure operation _still_ would potentially hop off the caller isolation. We introduced this change because the plan was the non-Sendable closure would run on the isolated parameter's isolation, but that's not actually the case: Instead, we can use the @execution(caller) on the function and closure, in order to guarantee there is no hop between those at all, and developers can trust that adding this cancellation handler will not cause any unexpected isolation changes and hops. The API was always documented to not hop as we execute the operation, so this brings the correct and expected behavior. resolves rdar://140110775 move to nonisolated(nonsending) [Concurrency] latest cancellation handler is emit into client; no ABI entry --- .../public/Concurrency/TaskCancellation.swift | 22 +++++- ...ation_handler_operation_does_not_hop.swift | 71 +++++++++++++++++++ .../stability-concurrency-abi.test | 5 +- utils/swift_snapshot_tool/Package.resolved | 15 ---- 4 files changed, 93 insertions(+), 20 deletions(-) create mode 100644 test/Concurrency/Runtime/cancellation_handler_operation_does_not_hop.swift delete mode 100644 utils/swift_snapshot_tool/Package.resolved diff --git a/stdlib/public/Concurrency/TaskCancellation.swift b/stdlib/public/Concurrency/TaskCancellation.swift index cd0a2bfaf5fb7..8d318b473fd3a 100644 --- a/stdlib/public/Concurrency/TaskCancellation.swift +++ b/stdlib/public/Concurrency/TaskCancellation.swift @@ -67,10 +67,26 @@ import Swift /// Therefore, if a cancellation handler must acquire a lock, other code should /// not cancel tasks or resume continuations while holding that lock. @available(SwiftStdlib 5.1, *) -#if !$Embedded -@backDeployed(before: SwiftStdlib 6.0) -#endif +@_alwaysEmitIntoClient +nonisolated(nonsending) public func withTaskCancellationHandler( + operation: nonisolated(nonsending) () async throws -> T, + onCancel handler: @Sendable () -> Void +) async rethrows -> T { + // unconditionally add the cancellation record to the task. + // if the task was already cancelled, it will be executed right away. + let record = unsafe _taskAddCancellationHandler(handler: handler) + defer { unsafe _taskRemoveCancellationHandler(record: record) } + + return try await operation() +} + +// Note: Deprecated version which would still hop if we did not close over an `isolated` parameter +// with the operation closure. Instead, we should do what the docs of this method promise - and not hop at all, +// by using the new nonisolated(nonsending) +@available(SwiftStdlib 5.1, *) +@_silgen_name("$ss27withTaskCancellationHandler9operation8onCancel9isolationxxyYaKXE_yyYbXEScA_pSgYitYaKlF") +public func _isolatedParam_withTaskCancellationHandler( operation: () async throws -> T, onCancel handler: @Sendable () -> Void, isolation: isolated (any Actor)? = #isolation diff --git a/test/Concurrency/Runtime/cancellation_handler_operation_does_not_hop.swift b/test/Concurrency/Runtime/cancellation_handler_operation_does_not_hop.swift new file mode 100644 index 0000000000000..ff9363de6921a --- /dev/null +++ b/test/Concurrency/Runtime/cancellation_handler_operation_does_not_hop.swift @@ -0,0 +1,71 @@ +// RUN: %target-run-simple-swift( -target %target-swift-5.1-abi-triple %import-libdispatch) | %FileCheck %s +// REQUIRES: concurrency +// REQUIRES: executable_test + +// REQUIRES: concurrency_runtime +// UNSUPPORTED: back_deployment_runtime +// UNSUPPORTED: freestanding + +//public func test(_ block: nonisolated(nonsending) () async -> T ) async { +// await block() +//} + +actor Canceller { + var hello: String = "checking..." + + func testFunc() async { + await withTaskCancellationHandler { + self.assertIsolated("wat in \(#function)!") + print("testFunc.withTaskCancellationHandler") // CHECK: testFunc.withTaskCancellationHandler + self.hello = "done!" + } onCancel: { + // noop + } + + // just a simple check to see we executed the closure + + await globalTestFunc() + } +} +func globalTestFunc(isolation: isolated (any Actor)? = #isolation) async { + isolation!.assertIsolated("wat in \(#function)!") + await withTaskCancellationHandler { + isolation!.assertIsolated("wat in \(#function)!") + print("globalTestFunc.withTaskCancellationHandler") // CHECK: globalTestFunc.withTaskCancellationHandler + } onCancel: { + // noop + } +} + +@MainActor +func testMainActor() async { + MainActor.preconditionIsolated("Expected main actor") + await withTaskCancellationHandler { + MainActor.preconditionIsolated("expected MainActor") + } onCancel: { + // noop + } +} + +func testMainActorIsolated(isolation: isolated (any Actor)? = #isolation) async { + isolation!.preconditionIsolated("Expected main actor") + MainActor.preconditionIsolated("Expected main actor") + await withTaskCancellationHandler { + print("_unsafeInheritExecutor_withTaskCancellationHandler") + MainActor.preconditionIsolated("expected MainActor") + } onCancel: { + // noop + } +} + +_ = await Canceller().testFunc() + +_ = await Task { @MainActor in + await testMainActor() +}.value + +_ = await Task { @MainActor in + await testMainActorIsolated() +}.value + +print("done") // CHECK: done \ No newline at end of file diff --git a/test/api-digester/stability-concurrency-abi.test b/test/api-digester/stability-concurrency-abi.test index 8e8e24505f24f..fa5566420f3d7 100644 --- a/test/api-digester/stability-concurrency-abi.test +++ b/test/api-digester/stability-concurrency-abi.test @@ -89,8 +89,9 @@ Func withCheckedThrowingContinuation(function:_:) has parameter 0 type change fr Func withCheckedThrowingContinuation(function:_:) has parameter 1 type change from (_Concurrency.CheckedContinuation<τ_0_0, any Swift.Error>) -> () to Swift.String // #isolation adoption for cancellation handlers; old APIs are kept ABI compatible -Func withTaskCancellationHandler(operation:onCancel:) has been renamed to Func withTaskCancellationHandler(operation:onCancel:isolation:) -Func withTaskCancellationHandler(operation:onCancel:) has mangled name changing from '_Concurrency.withTaskCancellationHandler(operation: () async throws -> A, onCancel: @Sendable () -> ()) async throws -> A' to '_Concurrency.withTaskCancellationHandler(operation: () async throws -> A, onCancel: @Sendable () -> (), isolation: isolated Swift.Optional) async throws -> A' +// but ABI checker does not understand the silgen_names on the ABi-compat APIs +Func withTaskCancellationHandler(operation:onCancel:) has mangled name changing from '_Concurrency.withTaskCancellationHandler(operation: () async throws -> A, onCancel: @Sendable () -> ()) async throws -> A' to '_Concurrency._isolatedParam_withTaskCancellationHandler(operation: () async throws -> A, onCancel: @Sendable () -> (), isolation: isolated Swift.Optional) async throws -> A' +Func withTaskCancellationHandler(operation:onCancel:) has been renamed to Func _isolatedParam_withTaskCancellationHandler(operation:onCancel:isolation:) // #isolated was adopted and the old methods kept: $ss31withCheckedThrowingContinuation8function_xSS_yScCyxs5Error_pGXEtYaKlF Func withCheckedContinuation(function:_:) has been renamed to Func withCheckedContinuation(isolation:function:_:) diff --git a/utils/swift_snapshot_tool/Package.resolved b/utils/swift_snapshot_tool/Package.resolved deleted file mode 100644 index 3dcc511149ffb..0000000000000 --- a/utils/swift_snapshot_tool/Package.resolved +++ /dev/null @@ -1,15 +0,0 @@ -{ - "originHash" : "80ba966d30db4d84324c2bda4f2b419b9e6fe208fa1f70080f74c7aa3d432a49", - "pins" : [ - { - "identity" : "swift-argument-parser", - "kind" : "remoteSourceControl", - "location" : "https://github.com/apple/swift-argument-parser", - "state" : { - "revision" : "41982a3656a71c768319979febd796c6fd111d5c", - "version" : "1.5.0" - } - } - ], - "version" : 3 -} From 9f1e063e99c7d797b2c1f6ae91aa4b97092afd51 Mon Sep 17 00:00:00 2001 From: Konrad 'ktoso' Malawski Date: Tue, 8 Jul 2025 20:15:21 +0900 Subject: [PATCH 2/2] [Concurrency] put test workaround in place until noniso-nonsend-throw clopsure inherits isolation --- .../cancellation_handler_operation_does_not_hop.swift | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/Concurrency/Runtime/cancellation_handler_operation_does_not_hop.swift b/test/Concurrency/Runtime/cancellation_handler_operation_does_not_hop.swift index ff9363de6921a..f4fdf8261f5d8 100644 --- a/test/Concurrency/Runtime/cancellation_handler_operation_does_not_hop.swift +++ b/test/Concurrency/Runtime/cancellation_handler_operation_does_not_hop.swift @@ -6,10 +6,6 @@ // UNSUPPORTED: back_deployment_runtime // UNSUPPORTED: freestanding -//public func test(_ block: nonisolated(nonsending) () async -> T ) async { -// await block() -//} - actor Canceller { var hello: String = "checking..." @@ -47,7 +43,10 @@ func testMainActor() async { } } +// FIXME: rdar://155313349 - nonisolated(nonsending) closure does not pick up isolated parameter when the closure is throwing func testMainActorIsolated(isolation: isolated (any Actor)? = #isolation) async { + return // FIXME: until rdar://155313349 is fixed + isolation!.preconditionIsolated("Expected main actor") MainActor.preconditionIsolated("Expected main actor") await withTaskCancellationHandler {