From 268c10c9989386efffeb8aa2226cf78bc371aa8a Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 7 Mar 2023 22:10:01 +0900 Subject: [PATCH 1/2] [Executors] improved executor precondition messages document the get active current executor method --- include/swift/Runtime/Concurrency.h | 16 ++++ .../CompatibilityOverrideConcurrency.def | 8 ++ stdlib/public/Concurrency/Actor.cpp | 32 +++++++ .../Concurrency/ExecutorAssertions.swift | 74 ++++++++++------ stdlib/public/Concurrency/Task.swift | 24 +++++ stdlib/public/Concurrency/TaskGroup.swift | 1 - .../actor_assert_precondition_executor.swift | 2 +- ...ft => actor_assume_executor_general.swift} | 88 ++++++++++++------- .../actor_assume_executor_main_async.swift | 31 +++++++ .../actor_assume_executor_main_sync.swift | 53 +++++++++++ .../actor_assume_executor_unknown.swift | 38 ++++++++ 11 files changed, 310 insertions(+), 57 deletions(-) rename test/Concurrency/Runtime/{actor_assume_executor.swift => actor_assume_executor_general.swift} (51%) create mode 100644 test/Concurrency/Runtime/actor_assume_executor_main_async.swift create mode 100644 test/Concurrency/Runtime/actor_assume_executor_main_sync.swift create mode 100644 test/Concurrency/Runtime/actor_assume_executor_unknown.swift diff --git a/include/swift/Runtime/Concurrency.h b/include/swift/Runtime/Concurrency.h index fb34a38bbcf01..418d44d71a178 100644 --- a/include/swift/Runtime/Concurrency.h +++ b/include/swift/Runtime/Concurrency.h @@ -891,6 +891,22 @@ ExecutorRef swift_task_getMainExecutor(void); SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift) bool swift_task_isCurrentExecutor(ExecutorRef executor); +SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift) +TypeNamePair swift_task_getExecutorRefTypeName(ExecutorRef executor); + +/// Since the `nullptr, 0` executor ref is a valid "generic executor" reference, +/// when we need to know "was the generic one actually set, or are we just defaulting to it +struct ExecutorActiveAndRef { + bool active; + ExecutorRef ref; +}; + +/// Different than `swift_task_getCurrentExecutor` in that it does not default to the generic executor. +/// This method is to be used if we are interested specifically IF we have an executor set in this context, +/// and if so, which one it is. If no executor is available in the current context, the `active` bool will be false. +SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift) +ExecutorActiveAndRef swift_task_getCurrentActiveExecutorRef(void); + SWIFT_EXPORT_FROM(swift_Concurrency) SWIFT_CC(swift) void swift_task_reportUnexpectedExecutor( const unsigned char *file, uintptr_t fileLength, bool fileIsASCII, diff --git a/stdlib/public/CompatibilityOverride/CompatibilityOverrideConcurrency.def b/stdlib/public/CompatibilityOverride/CompatibilityOverrideConcurrency.def index 44ebbb76a25a3..baa73141d9fce 100644 --- a/stdlib/public/CompatibilityOverride/CompatibilityOverrideConcurrency.def +++ b/stdlib/public/CompatibilityOverride/CompatibilityOverrideConcurrency.def @@ -124,6 +124,14 @@ OVERRIDE_ACTOR(task_isCurrentExecutor, bool, SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift), swift::, (ExecutorRef executor), (executor)) +OVERRIDE_ACTOR(task_getExecutorRefTypeName, TypeNamePair, + SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift), + swift::, (ExecutorRef executor), (executor)) + +OVERRIDE_ACTOR(task_getCurrentActiveExecutorRef, ExecutorActiveAndRef, + SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swift), + swift::, ,) + OVERRIDE_ACTOR(task_switch, void, SWIFT_EXPORT_FROM(swift_Concurrency), SWIFT_CC(swiftasync), swift::, (SWIFT_ASYNC_CONTEXT AsyncContext *resumeToContext, diff --git a/stdlib/public/Concurrency/Actor.cpp b/stdlib/public/Concurrency/Actor.cpp index 2025bf2178839..056360f1751d4 100644 --- a/stdlib/public/Concurrency/Actor.cpp +++ b/stdlib/public/Concurrency/Actor.cpp @@ -295,15 +295,47 @@ JobPriority swift::swift_task_getCurrentThreadPriority() { #endif } +SWIFT_CC(swift) +static TypeNamePair swift_task_getExecutorRefTypeNameImpl(ExecutorRef ref) { + TypeNamePair executorName; + if (ref.isDefaultActor()) { + auto defaultActorExecutorName = "DefaultActorExecutor"; + executorName = TypeNamePair{defaultActorExecutorName, strlen(defaultActorExecutorName)}; + } else if (ref.isMainExecutor()) { + auto mainActorExecutorName = "MainActorExecutor"; + executorName = TypeNamePair{mainActorExecutorName, strlen(mainActorExecutorName)}; + } else if (ref.isGeneric()) { + auto genericExecutorName = "GenericExecutor"; // TODO: what's a better name for it? + executorName = TypeNamePair{genericExecutorName, strlen(genericExecutorName)}; + } else { + HeapObject * identity = ref.getIdentity(); + assert(identity); + auto *metadata = identity->metadata; + executorName = swift_getTypeName(metadata, /*qualified=*/true); + } + + return executorName; +} + SWIFT_CC(swift) static bool swift_task_isCurrentExecutorImpl(ExecutorRef executor) { if (auto currentTracking = ExecutorTrackingInfo::current()) { return currentTracking->getActiveExecutor() == executor; } + // TODO(ktoso): wrong assumption; we can be on the main queue without being on main thread return executor.isMainExecutor() && isExecutingOnMainThread(); } +SWIFT_CC(swift) +static ExecutorActiveAndRef swift_task_getCurrentActiveExecutorRefImpl() { + if (auto currentTracking = ExecutorTrackingInfo::current()) { + return ExecutorActiveAndRef{true, currentTracking->getActiveExecutor()}; // FIXME; special case main and default + } + + return ExecutorActiveAndRef{false, ExecutorRef::generic()}; +} + /// Logging level for unexpected executors: /// 0 - no logging /// 1 - warn on each instance diff --git a/stdlib/public/Concurrency/ExecutorAssertions.swift b/stdlib/public/Concurrency/ExecutorAssertions.swift index d2d6bf55bdc4e..1ef13c3c1e7d5 100644 --- a/stdlib/public/Concurrency/ExecutorAssertions.swift +++ b/stdlib/public/Concurrency/ExecutorAssertions.swift @@ -34,7 +34,8 @@ import SwiftShims /// programming error. /// /// - Parameter executor: the expected current executor -@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9) +@available(SwiftStdlib 5.9, *) +@_transparent public func preconditionTaskOnExecutor( _ executor: some SerialExecutor, @@ -47,10 +48,9 @@ func preconditionTaskOnExecutor( let expectationCheck = _taskIsCurrentExecutor(executor.asUnownedSerialExecutor().executor) - /// TODO: implement the logic in-place perhaps rather than delegating to precondition()? precondition(expectationCheck, - // TODO: offer information which executor we actually got - "Incorrect actor executor assumption; Expected '\(executor)' executor. \(message())", + "Expected '\(_executorGetTypeName(executor.asUnownedSerialExecutor().executor))' executor, " + + "but was executing on '\(_executorGetCurrentActiveExecutorName())'.", file: file, line: line) // short-cut so we get the exact same failure reporting semantics } @@ -71,7 +71,8 @@ func preconditionTaskOnExecutor( /// programming error. /// /// - Parameter actor: the actor whose serial executor we expect to be the current executor -@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9) +@available(SwiftStdlib 5.9, *) +@_transparent public func preconditionTaskOnActorExecutor( _ actor: some Actor, @@ -84,10 +85,10 @@ func preconditionTaskOnActorExecutor( let expectationCheck = _taskIsCurrentExecutor(actor.unownedExecutor.executor) - // TODO: offer information which executor we actually got precondition(expectationCheck, - // TODO: figure out a way to get the typed repr out of the unowned executor - "Incorrect actor executor assumption; Expected '\(actor.unownedExecutor)' executor. \(message())", + "Expected same executor as actor '\(actor)' " + + "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " + + "but was executing on '\(_executorGetCurrentActiveExecutorName())'. \(message())", file: file, line: line) } @@ -108,7 +109,8 @@ func preconditionTaskOnActorExecutor( /// assumption is a serious programming error. /// /// - Parameter executor: the expected current executor -@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9) +@available(SwiftStdlib 5.9, *) +@_transparent public func assertTaskOnExecutor( _ executor: some SerialExecutor, @@ -120,10 +122,10 @@ func assertTaskOnExecutor( } guard _taskIsCurrentExecutor(executor.asUnownedSerialExecutor().executor) else { - // TODO: offer information which executor we actually got - let msg = "Incorrect actor executor assumption; Expected '\(executor)' executor. \(message())" - /// TODO: implement the logic in-place perhaps rather than delegating to precondition()? - assertionFailure(msg, file: file, line: line) + assertionFailure( + "Expected '\(_executorGetTypeName(executor.asUnownedSerialExecutor().executor))' executor, " + + "but was executing on '\(_executorGetCurrentActiveExecutorName())'.", + file: file, line: line) return } } @@ -143,7 +145,8 @@ func assertTaskOnExecutor( /// /// /// - Parameter actor: the actor whose serial executor we expect to be the current executor -@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9) +@available(SwiftStdlib 5.9, *) +@_transparent public func assertTaskOnActorExecutor( _ actor: some Actor, @@ -155,10 +158,11 @@ func assertTaskOnActorExecutor( } guard _taskIsCurrentExecutor(actor.unownedExecutor.executor) else { - // TODO: offer information which executor we actually got - // TODO: figure out a way to get the typed repr out of the unowned executor - let msg = "Incorrect actor executor assumption; Expected '\(actor.unownedExecutor)' executor. \(message())" - /// TODO: implement the logic in-place perhaps rather than delegating to precondition()? + let msg = + "Expected same executor as actor '\(actor)' " + + "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " + + "but was executing on '\(_executorGetCurrentActiveExecutorName())'. " + + "\(message())"; assertionFailure(msg, file: file, line: line) // short-cut so we get the exact same failure reporting semantics return } @@ -180,7 +184,7 @@ func assertTaskOnActorExecutor( /// if another actor uses the same serial executor--by using ``MainActor/sharedUnownedExecutor`` /// as its own ``Actor/unownedExecutor``--this check will succeed, as from a concurrency safety /// perspective, the serial executor guarantees mutual exclusion of those two actors. -@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9) +@available(SwiftStdlib 5.9, *) @_unavailableFromAsync(message: "await the call to the @MainActor closure directly") public func assumeOnMainActorExecutor( @@ -192,9 +196,13 @@ func assumeOnMainActorExecutor( /// This is guaranteed to be fatal if the check fails, /// as this is our "safe" version of this API. - guard _taskIsCurrentExecutor(Builtin.buildMainActorExecutorRef()) else { - // TODO: offer information which executor we actually got - fatalError("Incorrect actor executor assumption; Expected 'MainActor' executor.", file: file, line: line) + let mainExecutor = Builtin.buildMainActorExecutorRef() + + guard _taskIsCurrentExecutor(mainExecutor) else { + fatalError( + "Expected '\(_executorGetTypeName(mainExecutor))' executor, " + + "but was executing on '\(_executorGetCurrentActiveExecutorName())'.", + file: file, line: line) } // To do the unsafe cast, we have to pretend it's @escaping. @@ -218,7 +226,7 @@ func assumeOnMainActorExecutor( /// if another actor uses the same serial executor--by using that actor's ``Actor/unownedExecutor`` /// as its own ``Actor/unownedExecutor``--this check will succeed, as from a concurrency safety /// perspective, the serial executor guarantees mutual exclusion of those two actors. -@available(SwiftStdlib 5.9, *) // FIXME: use @backDeploy(before: SwiftStdlib 5.9) +@available(SwiftStdlib 5.9, *) @_unavailableFromAsync(message: "express the closure as an explicit function declared on the specified 'actor' instead") public func assumeOnActorExecutor( @@ -233,8 +241,11 @@ func assumeOnActorExecutor( /// as this is our "safe" version of this API. let executor: Builtin.Executor = actor.unownedExecutor.executor guard _taskIsCurrentExecutor(executor) else { - // TODO: offer information which executor we actually got - fatalError("Incorrect actor executor assumption; Expected same executor as \(actor).", file: file, line: line) + fatalError( + "Expected same executor as actor '\(actor)' " + + "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " + + "but was executing on '\(_executorGetCurrentActiveExecutorName())'.", + file: file, line: line) } // To do the unsafe cast, we have to pretend it's @escaping. @@ -245,5 +256,18 @@ func assumeOnActorExecutor( } } +/// Specifically get the name of the current *active* executor, without falling back to assuming the generic one. +/// I.e. if we're not executing within a task, we expect to have `` executor rather than the generic one. +@usableFromInline +@available(SwiftStdlib 5.9, *) +func _executorGetCurrentActiveExecutorName() -> String { + let (wasActive, ref) = _executorGetCurrentActiveExecutorRef() + guard wasActive else { + return "" + } + + return _executorGetTypeName(ref) +} + // TODO(ktoso): implement assume for distributed actors as well #endif // not SWIFT_STDLIB_TASK_TO_THREAD_MODEL_CONCURRENCY \ No newline at end of file diff --git a/stdlib/public/Concurrency/Task.swift b/stdlib/public/Concurrency/Task.swift index fb5fb6a9b3bfa..bf6f69647d254 100644 --- a/stdlib/public/Concurrency/Task.swift +++ b/stdlib/public/Concurrency/Task.swift @@ -1023,6 +1023,30 @@ func _taskCreateNullaryContinuationJob(priority: Int, continuation: Builtin.RawU @_silgen_name("swift_task_isCurrentExecutor") func _taskIsCurrentExecutor(_ executor: Builtin.Executor) -> Bool +@available(SwiftStdlib 5.9, *) +@usableFromInline +@_silgen_name("swift_task_getExecutorRefTypeName") +func _executorGetTypeNameRaw(_ executor: Builtin.Executor) -> (UnsafePointer?, Int) + +@available(SwiftStdlib 5.9, *) +@usableFromInline +func _executorGetTypeName(_ executor: Builtin.Executor) -> String { + let (stringPtr, count) = _executorGetTypeNameRaw(executor) + guard let stringPtr else { + return "" + } + + // FIXME: can't use this since it is internal in stdlib and we're in concurrency + // return String._fromUTF8Repairing( + // UnsafeBufferPointer(start: stringPtr, count: count)).0 + return String(cString: stringPtr) +} + +@available(SwiftStdlib 5.9, *) +@usableFromInline +@_silgen_name("swift_task_getCurrentActiveExecutorRef") +func _executorGetCurrentActiveExecutorRef() -> (Bool, Builtin.Executor) + @available(SwiftStdlib 5.1, *) @usableFromInline @_silgen_name("swift_task_reportUnexpectedExecutor") diff --git a/stdlib/public/Concurrency/TaskGroup.swift b/stdlib/public/Concurrency/TaskGroup.swift index c8a7930d27f17..5fbbed3697c99 100644 --- a/stdlib/public/Concurrency/TaskGroup.swift +++ b/stdlib/public/Concurrency/TaskGroup.swift @@ -632,7 +632,6 @@ public struct ThrowingTaskGroup { /// } catch { /// // other errors though we print and cancel the group, /// // and all of the remaining child tasks within it. - /// print("Error: \(error)") /// group.cancelAll() /// } /// } diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor.swift index 2506b8b37d5c4..e011cbe08b544 100644 --- a/test/Concurrency/Runtime/actor_assert_precondition_executor.swift +++ b/test/Concurrency/Runtime/actor_assert_precondition_executor.swift @@ -73,7 +73,7 @@ actor Someone { } tests.test("preconditionTaskOnActorExecutor(main): wrongly assume the main executor, from actor on other executor") { - expectCrashLater(withMessage: "Incorrect actor executor assumption; Expected 'MainActor' executor.") + expectCrashLater(withMessage: "Expected same executor as actor 'Swift.MainActor' ('MainActorExecutor'), but was executing on 'DefaultActorExecutor'.") await Someone().callCheckMainActor() } diff --git a/test/Concurrency/Runtime/actor_assume_executor.swift b/test/Concurrency/Runtime/actor_assume_executor_general.swift similarity index 51% rename from test/Concurrency/Runtime/actor_assume_executor.swift rename to test/Concurrency/Runtime/actor_assume_executor_general.swift index 17ae6236f2909..c487a02b3fc99 100644 --- a/test/Concurrency/Runtime/actor_assume_executor.swift +++ b/test/Concurrency/Runtime/actor_assume_executor_general.swift @@ -38,7 +38,7 @@ actor MainFriend { } } -func checkAssumeSomeone(someone: Someone) /* synchronous */ { +func checkAssumeDefaultExecutorSomeone(someone: DefaultExecutorSomeone) /* synchronous */ { // someone.something // can't access, would need a hop but we can't assumeOnActorExecutor(someone) { someone in let something = someone.something @@ -47,13 +47,13 @@ func checkAssumeSomeone(someone: Someone) /* synchronous */ { } } -actor Someone { +actor DefaultExecutorSomeone { func callCheckMainActor(echo: MainActorEcho) { checkAssumeMainActor(echo: echo) } - func callCheckSomeone() { - checkAssumeSomeone(someone: self) + func callCheckDefaultExecutorSomeone() { + checkAssumeDefaultExecutorSomeone(someone: self) } var something: String { @@ -61,29 +61,53 @@ actor Someone { } } -actor SomeonesFriend { - let someone: Someone +actor DefaultExecutorSomeonesFriend { + let someone: DefaultExecutorSomeone nonisolated var unownedExecutor: UnownedSerialExecutor { self.someone.unownedExecutor } - init(someone: Someone) { + init(someone: DefaultExecutorSomeone) { self.someone = someone } - func callCheckSomeone() { - checkAssumeSomeone(someone: someone) + func callCheckDefaultExecutorSomeone() { + checkAssumeDefaultExecutorSomeone(someone: someone) } } -actor CompleteStranger { - let someone: Someone - init(someone: Someone) { +final class InlineExecutor: SerialExecutor { + public func enqueue(_ job: UnownedJob) { + print("\(self): enqueue") + job.runSynchronously(on: self.asUnownedSerialExecutor()) + print("\(self): after run") + } + + public func asUnownedSerialExecutor() -> UnownedSerialExecutor { + return UnownedSerialExecutor(ordinary: self) + } +} + +let inlineExecutor = InlineExecutor() + +actor InlineExecutorActor { + let someone: DefaultExecutorSomeone + + // FIXME: why is making this a let cause crashes in swift_task_enqueueImpl + nonisolated var unownedExecutor: UnownedSerialExecutor { + inlineExecutor.asUnownedSerialExecutor() + } + + init(someone: DefaultExecutorSomeone) { self.someone = someone } - func callCheckSomeone() { - checkAssumeSomeone(someone: someone) + func callCheckDefaultExecutorSomeone() { + checkAssumeDefaultExecutorSomeone(someone: someone) + } + + func checkMainActor() { + preconditionTaskOnActorExecutor(MainActor.shared) } } @@ -94,17 +118,17 @@ final class MainActorEcho { } } +/// Crash tests @main struct Main { static func main() async { let tests = TestSuite("AssumeActorExecutor") - let echo = MainActorEcho() if #available(SwiftStdlib 5.9, *) { // === MainActor -------------------------------------------------------- tests.test("assumeOnMainActorExecutor: assume the main executor, from 'main() async'") { - await checkAssumeMainActor(echo: echo) + await checkAssumeMainActor(echo: echo) // purposefully await here, forcing a hop to MainActor } tests.test("assumeOnMainActorExecutor: assume the main executor, from MainActor method") { @@ -116,37 +140,41 @@ final class MainActorEcho { } tests.test("assumeOnMainActorExecutor: wrongly assume the main executor, from actor on other executor") { - expectCrashLater(withMessage: "Incorrect actor executor assumption; Expected 'MainActor' executor.") - await Someone().callCheckMainActor(echo: echo) + expectCrashLater(withMessage: "Expected 'MainActorExecutor' executor, but was executing on 'DefaultActorExecutor'.") + await DefaultExecutorSomeone().callCheckMainActor(echo: echo) } // === some Actor ------------------------------------------------------- - let someone = Someone() + let someone = DefaultExecutorSomeone() + tests.test("assumeOnActorExecutor: wrongly assume someone's executor, from 'main() async'") { - expectCrashLater(withMessage: "Incorrect actor executor assumption; Expected same executor as a.Someone.") - checkAssumeSomeone(someone: someone) + expectCrashLater(withMessage: "Expected same executor as actor 'a.DefaultExecutorSomeone' ('DefaultActorExecutor'), but was executing on 'GenericExecutor'.") + checkAssumeDefaultExecutorSomeone(someone: someone) } tests.test("assumeOnActorExecutor: wrongly assume someone's executor, from MainActor method") { - expectCrashLater(withMessage: "Incorrect actor executor assumption; Expected same executor as a.Someone.") - checkAssumeSomeone(someone: someone) + expectCrashLater(withMessage: "Expected same executor as actor 'a.DefaultExecutorSomeone' ('DefaultActorExecutor'), but was executing on 'GenericExecutor'.") + checkAssumeDefaultExecutorSomeone(someone: someone) } - tests.test("assumeOnActorExecutor: assume someone's executor, from Someone") { - await someone.callCheckSomeone() + tests.test("assumeOnActorExecutor: assume someone's executor, from DefaultExecutorSomeone") { + await someone.callCheckDefaultExecutorSomeone() } - tests.test("assumeOnActorExecutor: assume someone's executor, from actor on the Someone.unownedExecutor") { - await SomeonesFriend(someone: someone).callCheckSomeone() + tests.test("assumeOnActorExecutor: assume someone's executor, from actor on the DefaultExecutorSomeone.unownedExecutor") { + await DefaultExecutorSomeonesFriend(someone: someone).callCheckDefaultExecutorSomeone() } tests.test("assumeOnActorExecutor: wrongly assume the main executor, from actor on other executor") { - expectCrashLater(withMessage: "Incorrect actor executor assumption; Expected same executor as a.Someone.") - await CompleteStranger(someone: someone).callCheckSomeone() + expectCrashLater(withMessage: "Expected same executor as actor 'a.DefaultExecutorSomeone' ('DefaultActorExecutor'), but was executing on 'a.InlineExecutor'.") + await InlineExecutorActor(someone: someone).callCheckDefaultExecutorSomeone() } - + tests.test("assumeOnActorExecutor: wrongly assume the main executor, on custom executor") { + expectCrashLater(withMessage: "Expected same executor as actor 'Swift.MainActor' ('MainActorExecutor'), but was executing on 'a.InlineExecutor'.") + await InlineExecutorActor(someone: someone).checkMainActor() + } } await runAllTestsAsync() diff --git a/test/Concurrency/Runtime/actor_assume_executor_main_async.swift b/test/Concurrency/Runtime/actor_assume_executor_main_async.swift new file mode 100644 index 0000000000000..21fe99a42b48c --- /dev/null +++ b/test/Concurrency/Runtime/actor_assume_executor_main_async.swift @@ -0,0 +1,31 @@ +// RUN: %empty-directory(%t) +// RUN: %target-build-swift -Xfrontend -disable-availability-checking -parse-as-library %s -o %t/a.out +// RUN: %target-codesign %t/a.out +// RUN: %target-run %t/a.out + +// REQUIRES: executable_test +// REQUIRES: concurrency +// REQUIRES: concurrency_runtime +// UNSUPPORTED: back_deployment_runtime + +// UNSUPPORTED: back_deploy_concurrency +// UNSUPPORTED: use_os_stdlib +// UNSUPPORTED: freestanding + +//import StdlibUnittest + +func checkAssumeMainActor() /* synchronous! */ { + assumeOnMainActorExecutor { + print("OK") + } +} + +/// Crash tests, in order to make sure we report the expected diagnostics. +@main struct Main { + static func main() async { + if #available(SwiftStdlib 5.9, *) { + // running directly without the Unittest suite as that would cause a hop off the main actor + checkAssumeMainActor() + } + } +} diff --git a/test/Concurrency/Runtime/actor_assume_executor_main_sync.swift b/test/Concurrency/Runtime/actor_assume_executor_main_sync.swift new file mode 100644 index 0000000000000..d864a9e682a8e --- /dev/null +++ b/test/Concurrency/Runtime/actor_assume_executor_main_sync.swift @@ -0,0 +1,53 @@ +// RUN: %empty-directory(%t) +// RUN: %target-build-swift -Xfrontend -disable-availability-checking -parse-as-library %s -o %t/a.out +// RUN: %target-codesign %t/a.out +// RUN: %target-run %t/a.out + +// REQUIRES: executable_test +// REQUIRES: concurrency +// REQUIRES: concurrency_runtime +// UNSUPPORTED: back_deployment_runtime + +// UNSUPPORTED: back_deploy_concurrency +// UNSUPPORTED: use_os_stdlib +// UNSUPPORTED: freestanding + +import StdlibUnittest + +func checkAssumeMainActor() /* synchronous! */ { + assumeOnMainActorExecutor { + print("OK") + } +} + +@main struct Main { + static func main() /* synchronous! */ { + let tests = TestSuite("AssumeActorExecutor") + + if #available(SwiftStdlib 5.9, *) { + // === MainActor -------------------------------------------------------- + + tests.test("assumeOnMainActorExecutor: assume the main executor, from 'main()'") { + preconditionTaskOnActorExecutor(MainActor.shared) + checkAssumeMainActor() + } + + tests.test("assumeOnMainActorExecutor: assume the main executor, from 'Task{}' inheriting MainActor isolation") { + Task { + // we properly inferred MainActor isolation + preconditionTaskOnActorExecutor(MainActor.shared) + checkAssumeMainActor() + } + } + + tests.test("assumeOnMainActorExecutor: assume the main executor, from 'Task{ @MainActor in }' isolation") { + Task { + preconditionTaskOnActorExecutor(MainActor.shared) + checkAssumeMainActor() + } + } + } + + runAllTests() // run synchronously + } +} diff --git a/test/Concurrency/Runtime/actor_assume_executor_unknown.swift b/test/Concurrency/Runtime/actor_assume_executor_unknown.swift new file mode 100644 index 0000000000000..5cbe0ba8caf6e --- /dev/null +++ b/test/Concurrency/Runtime/actor_assume_executor_unknown.swift @@ -0,0 +1,38 @@ +// RUN: %empty-directory(%t) +// RUN: %target-build-swift -Xfrontend -disable-availability-checking -parse-as-library %s -o %t/a.out +// RUN: %target-codesign %t/a.out +// RUN: %target-run %t/a.out + +// REQUIRES: executable_test +// REQUIRES: concurrency +// REQUIRES: concurrency_runtime +// REQUIRES: libdispatch +// UNSUPPORTED: back_deployment_runtime + +// UNSUPPORTED: back_deploy_concurrency +// UNSUPPORTED: use_os_stdlib +// UNSUPPORTED: freestanding + +import StdlibUnittest +import Dispatch + +@main struct Main { + static func main() async { + let tests = TestSuite("AssumeActorExecutor") + let q = DispatchQueue(label: "queue") + + if #available(SwiftStdlib 5.9, *) { + tests.test("assumeOnMainActorExecutor: wrongly assume the main executor, from DispatchQueue") { + expectCrashLater(withMessage: "Expected same executor as actor 'Swift.MainActor' ('MainActorExecutor'), but was executing on ''.") + await withCheckedContinuation { cc in + q.async { + preconditionTaskOnActorExecutor(MainActor.shared) + cc.resume() + } + } + } + } + + await runAllTestsAsync() // run synchronously + } +} From 9aab398ae3fbf38b6ba33dd7486bc51e118b5dc7 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Thu, 9 Mar 2023 17:13:40 +0900 Subject: [PATCH 2/2] [Executors] Specialized message for failed assert between 2 default actors fix missing default initializer for ExecutorActiveAndRef --- include/swift/Runtime/Concurrency.h | 12 ++- .../Concurrency/ExecutorAssertions.swift | 81 ++++++++++++++----- .../actor_assume_executor_general.swift | 18 +++++ 3 files changed, 89 insertions(+), 22 deletions(-) diff --git a/include/swift/Runtime/Concurrency.h b/include/swift/Runtime/Concurrency.h index 418d44d71a178..ab295d3208ed5 100644 --- a/include/swift/Runtime/Concurrency.h +++ b/include/swift/Runtime/Concurrency.h @@ -897,8 +897,16 @@ TypeNamePair swift_task_getExecutorRefTypeName(ExecutorRef executor); /// Since the `nullptr, 0` executor ref is a valid "generic executor" reference, /// when we need to know "was the generic one actually set, or are we just defaulting to it struct ExecutorActiveAndRef { - bool active; - ExecutorRef ref; + const bool active; + const ExecutorRef ref; + + ExecutorActiveAndRef() : + active(false), + ref(ExecutorRef::generic()) {} + + ExecutorActiveAndRef(bool active, ExecutorRef ref) : + active(active), + ref(ref) {} }; /// Different than `swift_task_getCurrentExecutor` in that it does not default to the generic executor. diff --git a/stdlib/public/Concurrency/ExecutorAssertions.swift b/stdlib/public/Concurrency/ExecutorAssertions.swift index 1ef13c3c1e7d5..002fe7f1d1700 100644 --- a/stdlib/public/Concurrency/ExecutorAssertions.swift +++ b/stdlib/public/Concurrency/ExecutorAssertions.swift @@ -49,8 +49,7 @@ func preconditionTaskOnExecutor( let expectationCheck = _taskIsCurrentExecutor(executor.asUnownedSerialExecutor().executor) precondition(expectationCheck, - "Expected '\(_executorGetTypeName(executor.asUnownedSerialExecutor().executor))' executor, " + - "but was executing on '\(_executorGetCurrentActiveExecutorName())'.", + makeUnexpectedExecutorMessage(executor: executor.asUnownedSerialExecutor().executor, message: message()), file: file, line: line) // short-cut so we get the exact same failure reporting semantics } @@ -86,9 +85,7 @@ func preconditionTaskOnActorExecutor( let expectationCheck = _taskIsCurrentExecutor(actor.unownedExecutor.executor) precondition(expectationCheck, - "Expected same executor as actor '\(actor)' " + - "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " + - "but was executing on '\(_executorGetCurrentActiveExecutorName())'. \(message())", + makeUnexpectedExecutorMessage(actor: actor, message: message()), file: file, line: line) } @@ -123,8 +120,7 @@ func assertTaskOnExecutor( guard _taskIsCurrentExecutor(executor.asUnownedSerialExecutor().executor) else { assertionFailure( - "Expected '\(_executorGetTypeName(executor.asUnownedSerialExecutor().executor))' executor, " + - "but was executing on '\(_executorGetCurrentActiveExecutorName())'.", + makeUnexpectedExecutorMessage(executor: executor.asUnownedSerialExecutor().executor, message: message()), file: file, line: line) return } @@ -158,12 +154,9 @@ func assertTaskOnActorExecutor( } guard _taskIsCurrentExecutor(actor.unownedExecutor.executor) else { - let msg = - "Expected same executor as actor '\(actor)' " + - "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " + - "but was executing on '\(_executorGetCurrentActiveExecutorName())'. " + - "\(message())"; - assertionFailure(msg, file: file, line: line) // short-cut so we get the exact same failure reporting semantics + assertionFailure( + makeUnexpectedExecutorMessage(actor: actor, message: message()), + file: file, line: line) return } } @@ -200,8 +193,7 @@ func assumeOnMainActorExecutor( guard _taskIsCurrentExecutor(mainExecutor) else { fatalError( - "Expected '\(_executorGetTypeName(mainExecutor))' executor, " + - "but was executing on '\(_executorGetCurrentActiveExecutorName())'.", + makeUnexpectedExecutorMessage(executor: mainExecutor, message: nil), file: file, line: line) } @@ -241,10 +233,7 @@ func assumeOnActorExecutor( /// as this is our "safe" version of this API. let executor: Builtin.Executor = actor.unownedExecutor.executor guard _taskIsCurrentExecutor(executor) else { - fatalError( - "Expected same executor as actor '\(actor)' " + - "('\(_executorGetTypeName(actor.unownedExecutor.executor))'), " + - "but was executing on '\(_executorGetCurrentActiveExecutorName())'.", + fatalError(makeUnexpectedExecutorMessage(actor: actor, message: nil), file: file, line: line) } @@ -260,7 +249,7 @@ func assumeOnActorExecutor( /// I.e. if we're not executing within a task, we expect to have `` executor rather than the generic one. @usableFromInline @available(SwiftStdlib 5.9, *) -func _executorGetCurrentActiveExecutorName() -> String { +internal func _executorGetCurrentActiveExecutorName() -> String { let (wasActive, ref) = _executorGetCurrentActiveExecutorRef() guard wasActive else { return "" @@ -269,5 +258,57 @@ func _executorGetCurrentActiveExecutorName() -> String { return _executorGetTypeName(ref) } +@usableFromInline +@available(SwiftStdlib 5.9, *) +internal func makeUnexpectedExecutorMessage(actor: some Actor, message: String?) -> String { + let expectedExecutor = _executorGetTypeName(actor.unownedExecutor.executor) + let gotExecutor = _executorGetCurrentActiveExecutorName() + + let sameExecutorNameClarification: String + if expectedExecutor == gotExecutor { + sameExecutorNameClarification = " of a different actor" + } else { + sameExecutorNameClarification = "" + } + + let trailingMessage: String + if let message { + trailingMessage = " \(message)" + } else { + trailingMessage = "" + } + + return "Expected same executor as actor '\(actor)' " + + "('\(expectedExecutor)'), " + + "but was executing on '\(gotExecutor)'" + + "\(sameExecutorNameClarification)." + + "\(trailingMessage)" +} + +@usableFromInline +@available(SwiftStdlib 5.9, *) +internal func makeUnexpectedExecutorMessage(executor: Builtin.Executor, message: String?) -> String { + let expectedExecutor = _executorGetTypeName(executor) + let gotExecutor = _executorGetCurrentActiveExecutorName() + + let sameExecutorNameClarification: String + if expectedExecutor == gotExecutor { + sameExecutorNameClarification = " of a different actor" + } else { + sameExecutorNameClarification = "" + } + + let trailingMessage: String + if let message { + trailingMessage = " \(message)" + } else { + trailingMessage = "" + } + + return "Expected '\(expectedExecutor)' executor, " + + "but was executing on '\(gotExecutor)'" + + "\(sameExecutorNameClarification)." +} + // TODO(ktoso): implement assume for distributed actors as well #endif // not SWIFT_STDLIB_TASK_TO_THREAD_MODEL_CONCURRENCY \ No newline at end of file diff --git a/test/Concurrency/Runtime/actor_assume_executor_general.swift b/test/Concurrency/Runtime/actor_assume_executor_general.swift index c487a02b3fc99..f9623daa39787 100644 --- a/test/Concurrency/Runtime/actor_assume_executor_general.swift +++ b/test/Concurrency/Runtime/actor_assume_executor_general.swift @@ -61,6 +61,15 @@ actor DefaultExecutorSomeone { } } +actor DefaultExecutorSomeoneElse { + func checkSameExecutorAs(other: DefaultExecutorSomeone) { + // this is expected to fail + assumeOnActorExecutor(other) { isolatedOther in + let s: String = isolatedOther.something // ok, if the runtime check were to pass + } + } +} + actor DefaultExecutorSomeonesFriend { let someone: DefaultExecutorSomeone nonisolated var unownedExecutor: UnownedSerialExecutor { @@ -175,6 +184,15 @@ final class MainActorEcho { expectCrashLater(withMessage: "Expected same executor as actor 'Swift.MainActor' ('MainActorExecutor'), but was executing on 'a.InlineExecutor'.") await InlineExecutorActor(someone: someone).checkMainActor() } + + tests.test("assumeOnActorExecutor: wrongly assume 'same executor' as a different default actor, from a different default actor") { + // TODO: this message is not very helpful, and we should try to print the exact default actor we executed on, + // however, this is tricky due to the fact a default actor can be destroyed while executing on it, so we + // must take extra care for how we'd retain and print such default actor. rdar://106487242 + expectCrashLater(withMessage: "Expected same executor as actor 'a.DefaultExecutorSomeone' ('DefaultActorExecutor'), but was executing on 'DefaultActorExecutor' of a different actor.") + let someone = DefaultExecutorSomeone() + await DefaultExecutorSomeoneElse().checkSameExecutorAs(other: someone) + } } await runAllTestsAsync()