From 481c928d678e9451ed8ff1af205bb6ab55efdaab Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 21 May 2024 10:14:35 +0900 Subject: [PATCH 1/8] [Concurrency] Fix missing return in legacy isSameExecutor mode detection resolves rdar://128425368 resolves rdar://127400013 --- stdlib/public/Concurrency/Actor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/public/Concurrency/Actor.cpp b/stdlib/public/Concurrency/Actor.cpp index abb8b948be036..605fe0ec97670 100644 --- a/stdlib/public/Concurrency/Actor.cpp +++ b/stdlib/public/Concurrency/Actor.cpp @@ -339,7 +339,7 @@ static IsCurrentExecutorCheckMode isCurrentExecutorMode = // these symbols defined bool swift_bincompat_useLegacyNonCrashingExecutorChecks() { #if !SWIFT_CONCURRENCY_EMBEDDED - swift::runtime::bincompat:: + return swift::runtime::bincompat:: swift_bincompat_useLegacyNonCrashingExecutorChecks(); #endif return false; From 7790609fc83029992cba3cb75b6f7453a23d287c Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Wed, 22 May 2024 11:04:47 +0900 Subject: [PATCH 2/8] [Concurrency] Futher prevent crashes in legacy mode of isCurrentExecutor --- stdlib/public/Concurrency/Actor.cpp | 172 +++++++++++------- stdlib/public/runtime/Bincompat.cpp | 13 ++ .../public/runtime/EnvironmentVariables.def | 4 +- ...olated_bincompat_crash_swift_6_mode.swift} | 33 ++-- ...olated_bincompat_nocrash_legacy_mode.swift | 70 +++++++ ..._dispatch_dispatchMain_swift_6_mode.swift} | 6 +- ..._checkIsolated_dispatch_swift6_mode.swift} | 2 +- ...Isolated_main_customMain_swift6_mode.swift | 72 ++++++++ ..._executor_checkIsolated_swift6_mode.swift} | 5 +- ..._precondition_executor_default_mode.swift} | 2 +- .../preconcurrency_conformances.swift | 36 +++- ...cy_conformances_with_disabled_checks.swift | 8 + ...formances_with_legacy_isSameExecutor.swift | 105 +++++++++++ 13 files changed, 429 insertions(+), 99 deletions(-) rename test/Concurrency/Runtime/{actor_assert_precondition_executor_checkIsolated_bincompat.swift => actor_assert_precondition_executor_checkIsolated_bincompat_crash_swift_6_mode.swift} (61%) create mode 100644 test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat_nocrash_legacy_mode.swift rename test/Concurrency/Runtime/{actor_assert_precondition_executor_checkIsolated_dispatch_dispatchMain.swift => actor_assert_precondition_executor_checkIsolated_dispatch_dispatchMain_swift_6_mode.swift} (74%) rename test/Concurrency/Runtime/{actor_assert_precondition_executor_checkIsolated_dispatch.swift => actor_assert_precondition_executor_checkIsolated_dispatch_swift6_mode.swift} (97%) create mode 100644 test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_main_customMain_swift6_mode.swift rename test/Concurrency/Runtime/{actor_assert_precondition_executor_checkIsolated.swift => actor_assert_precondition_executor_checkIsolated_swift6_mode.swift} (86%) rename test/Concurrency/Runtime/{actor_assert_precondition_executor.swift => actor_assert_precondition_executor_default_mode.swift} (99%) create mode 100644 test/Interpreter/preconcurrency_conformances_with_legacy_isSameExecutor.swift diff --git a/stdlib/public/Concurrency/Actor.cpp b/stdlib/public/Concurrency/Actor.cpp index 605fe0ec97670..e1558da08a6ff 100644 --- a/stdlib/public/Concurrency/Actor.cpp +++ b/stdlib/public/Concurrency/Actor.cpp @@ -323,7 +323,7 @@ bool _task_serialExecutor_isSameExclusiveExecutionContext( enum IsCurrentExecutorCheckMode: unsigned { /// The default mode when an app was compiled against "new" enough SDK. /// It allows crashing in isCurrentExecutor, and calls into `checkIsolated`. - Default_UseCheckIsolated_AllowCrash, + Swift6_UseCheckIsolated_AllowCrash, /// Legacy mode; Primarily to support old applications which used data race /// detector with "warning" mode, which is no longer supported. When such app /// is re-compiled against a new SDK, it will see crashes in what was @@ -332,17 +332,18 @@ enum IsCurrentExecutorCheckMode: unsigned { Legacy_NoCheckIsolated_NonCrashing, }; static IsCurrentExecutorCheckMode isCurrentExecutorMode = - Default_UseCheckIsolated_AllowCrash; + Swift6_UseCheckIsolated_AllowCrash; // Shimming call to Swift runtime because Swift Embedded does not have -// these symbols defined +// these symbols defined. bool swift_bincompat_useLegacyNonCrashingExecutorChecks() { #if !SWIFT_CONCURRENCY_EMBEDDED return swift::runtime::bincompat:: swift_bincompat_useLegacyNonCrashingExecutorChecks(); -#endif +#else return false; +#endif } // Check override of executor checking mode. @@ -352,19 +353,17 @@ static void checkIsCurrentExecutorMode(void *context) { // Potentially, override the platform detected mode, primarily used in tests. #if SWIFT_STDLIB_HAS_ENVIRON - if (const char *modeStr = - runtime::environment::concurrencyIsCurrentExecutorLegacyModeOverride()) { - if (modeStr) { - if (strcmp(modeStr, "nocrash") == 0) { - useLegacyMode = true; - } else if (strcmp(modeStr, "crash") == 0) { - useLegacyMode = false; - } // else, just use the platform detected mode - } + if (const char *modeStr = runtime::environment::concurrencyIsCurrentExecutorLegacyModeOverride()) { + if (strcmp(modeStr, "nocrash") == 0 || strcmp(modeStr, "legacy") == 0) { + useLegacyMode = true; + } else if (strcmp(modeStr, "crash") == 0 || strcmp(modeStr, "swift6") == 0) { + useLegacyMode = false; + } // else, just use the platform detected mode } #endif // SWIFT_STDLIB_HAS_ENVIRON + isCurrentExecutorMode = useLegacyMode ? Legacy_NoCheckIsolated_NonCrashing - : Default_UseCheckIsolated_AllowCrash; + : Swift6_UseCheckIsolated_AllowCrash; } SWIFT_CC(swift) @@ -373,6 +372,12 @@ static bool swift_task_isCurrentExecutorImpl(SerialExecutorRef expectedExecutor) // To support old applications on apple platforms which assumed this call // does not crash, try to use a more compatible mode for those apps. + // + // We only allow returning `false` directly from this function when operating + // in 'Legacy_NoCheckIsolated_NonCrashing' mode. If allowing crashes, we + // instead must call into 'checkIsolated' or crash directly. + // + // Whenever we confirm an executor equality, we can return true, in any mode. static swift::once_t checkModeToken; swift::once(checkModeToken, checkIsCurrentExecutorMode, nullptr); @@ -382,20 +387,27 @@ static bool swift_task_isCurrentExecutorImpl(SerialExecutorRef expectedExecutor) // the expected executor however, so we need to try a bit harder before // we fail. - // Are we expecting the main executor and are using the main thread? - if (expectedExecutor.isMainExecutor() && isExecutingOnMainThread()) { - // Due to compatibility with pre-checkIsolated code, we cannot remove - // this special handling. CheckIsolated can handle this if the expected - // executor is the main queue / main executor, however, if we cannot call - // checkIsolated we cannot rely on it to handle this. - // TODO: consider removing this branch when `useCrashingCheckIsolated=true` - return true; + // Legacy special handling the main executor by detecting the main thread. + // + // When 'checkIsolated' is available it will perform a dispatch queue assertion + // against the main queue, potentially resulting in a crash (expected). + // + // In legacy mode, we cannot allow crashes here, and therefore we keep the + // special best-effort handling of the "main thread". + if (isCurrentExecutorMode == Legacy_NoCheckIsolated_NonCrashing) { + if (expectedExecutor.isMainExecutor() && isExecutingOnMainThread()) { + return true; + } } + // We cannot use 'complexEquality' as it requires two executor instances, + // and we do not have a 'current' executor here. + // Otherwise, as last resort, let the expected executor check using // external means, as it may "know" this thread is managed by it etc. - if (isCurrentExecutorMode == Default_UseCheckIsolated_AllowCrash) { - swift_task_checkIsolated(expectedExecutor); + if (isCurrentExecutorMode == Swift6_UseCheckIsolated_AllowCrash) { + swift_task_checkIsolated(expectedExecutor); // will crash if not same context + // checkIsolated did not crash, so we are on the right executor, after all! return true; } @@ -418,46 +430,50 @@ static bool swift_task_isCurrentExecutorImpl(SerialExecutorRef expectedExecutor) return true; } - // If the expected executor is "default" then we should have matched - // by pointer equality already with the current executor. - if (expectedExecutor.isDefaultActor()) { - // If the expected executor is a default actor, it makes no sense to try - // the 'checkIsolated' call, it must be equal to the other actor, or it is - // not the same isolation domain. - swift_Concurrency_fatalError(0, "Incorrect actor executor assumption"); - return false; - } - - if (expectedExecutor.isMainExecutor() && !currentExecutor.isMainExecutor()) { - // TODO: Invoke checkIsolated() on "main" SerialQueue once it implements `checkIsolated`, otherwise messages will be sub-par and hard to address - swift_Concurrency_fatalError(0, "Incorrect actor executor assumption; Expected MainActor executor"); - return false; - } else if (!expectedExecutor.isMainExecutor() && currentExecutor.isMainExecutor()) { - // TODO: Invoke checkIsolated() on "main" SerialQueue once it implements `checkIsolated`, otherwise messages will be sub-par and hard to address - swift_Concurrency_fatalError(0, "Incorrect actor executor assumption; Expected not-MainActor executor"); - return false; - } - - if (expectedExecutor.isComplexEquality()) { - if (!swift_compareWitnessTables( - reinterpret_cast(currentExecutor.getSerialExecutorWitnessTable()), - reinterpret_cast(expectedExecutor.getSerialExecutorWitnessTable()))) { - // different witness table, we cannot invoke complex equality call + // Only in legacy mode: + // We check if the current xor expected executor are the main executor. + // If so only one of them is, we know that WITHOUT 'checkIsolated' or invoking + // 'dispatch_assert_queue' we cannot be truly sure the expected/current truly + // are "on the same queue". There exists no non-crashing API to check this, + // so we PESSIMISTICALLY return false here. + // + // In Swift6 mode: + // We don't do this naive check, because we'll fall back to + // `expected.checkIsolated()` which, if it is the main executor, will invoke + // the crashing 'dispatch_assert_queue(main queue)' which will either crash + // or confirm we actually are on the main queue; or the custom expected + // executor has a chance to implement a similar queue check. + if (isCurrentExecutorMode == Legacy_NoCheckIsolated_NonCrashing) { + if ((expectedExecutor.isMainExecutor() && !currentExecutor.isMainExecutor()) || + (!expectedExecutor.isMainExecutor() && currentExecutor.isMainExecutor())) { return false; } + } - // Avoid passing nulls to Swift for the isSame check: - if (!currentExecutor.getIdentity() || !expectedExecutor.getIdentity()) { - return false; + // Complex equality means that if two executors of the same type have some + // special logic to check if they are "actually the same". + if (expectedExecutor.isComplexEquality()) { + if (currentExecutor.getIdentity() && + expectedExecutor.getIdentity() && + swift_compareWitnessTables( + reinterpret_cast( + currentExecutor.getSerialExecutorWitnessTable()), + reinterpret_cast( + expectedExecutor.getSerialExecutorWitnessTable()))) { + + auto isSameExclusiveExecutionContextResult = + _task_serialExecutor_isSameExclusiveExecutionContext( + currentExecutor.getIdentity(), expectedExecutor.getIdentity(), + swift_getObjectType(currentExecutor.getIdentity()), + expectedExecutor.getSerialExecutorWitnessTable()); + + // if the 'isSameExclusiveExecutionContext' returned true we trust + // it and return; if it was false, we need to give checkIsolated another + // chance to check. + if (isSameExclusiveExecutionContextResult) { + return true; + } // else, we must give 'checkIsolated' a last chance to verify isolation } - - auto result = _task_serialExecutor_isSameExclusiveExecutionContext( - currentExecutor.getIdentity(), - expectedExecutor.getIdentity(), - swift_getObjectType(currentExecutor.getIdentity()), - expectedExecutor.getSerialExecutorWitnessTable()); - - return result; } // This provides a last-resort check by giving the expected SerialExecutor the @@ -480,21 +496,22 @@ static bool swift_task_isCurrentExecutorImpl(SerialExecutorRef expectedExecutor) // Note that this only works because the closure in assumeIsolated is // synchronous, and will not cause suspensions, as that would require the // presence of a Task. - // compat_invoke_swift_task_checkIsolated(expectedExecutor); - if (isCurrentExecutorMode == Default_UseCheckIsolated_AllowCrash) { - swift_task_checkIsolated(expectedExecutor); + if (isCurrentExecutorMode == Swift6_UseCheckIsolated_AllowCrash) { + swift_task_checkIsolated(expectedExecutor); // will crash if not same context + // The checkIsolated call did not crash, so we are on the right executor. return true; } - // Using legacy mode, if no explicit executor match worked, we assume `false` + // In the end, since 'checkIsolated' could not be used, so we must assume + // that the executors are not the same context. assert(isCurrentExecutorMode == Legacy_NoCheckIsolated_NonCrashing); return false; } /// Logging level for unexpected executors: -/// 0 - no logging -/// 1 - warn on each instance +/// 0 - no logging -- will be IGNORED when Swift6 mode of isCurrentExecutor is used +/// 1 - warn on each instance -- will be IGNORED when Swift6 mode of isCurrentExecutor is used /// 2 - fatal error /// /// NOTE: The default behavior on Apple platforms depends on the SDK version @@ -511,9 +528,28 @@ static void checkUnexpectedExecutorLogLevel(void *context) { if (!levelStr) return; + auto isCurrentExecutorLegacyMode = + swift_bincompat_useLegacyNonCrashingExecutorChecks(); + long level = strtol(levelStr, nullptr, 0); - if (level >= 0 && level < 3) - unexpectedExecutorLogLevel = level; + if (level >= 0 && level < 3) { + if (isCurrentExecutorLegacyMode) { + // legacy mode permits doing nothing or just logging, since the method + // used to perform the check itself is not going to crash: + unexpectedExecutorLogLevel = level; + } else { + // We are in swift6/crash mode of isCurrentExecutor which means that + // rather than returning false, that method will always CRASH when an + // executor mismatch is discovered. + // + // Thus, for clarity, we set this mode also to crashing, as runtime should + // not expect to be able to get any logging or ignoring done. In practice, + // the crash would happen before logging or "ignoring", but this should + // help avoid confusing situations like "I thought it should log" when + // debugging the runtime. + unexpectedExecutorLogLevel = 2; + } + } #endif // SWIFT_STDLIB_HAS_ENVIRON } diff --git a/stdlib/public/runtime/Bincompat.cpp b/stdlib/public/runtime/Bincompat.cpp index 31175da758ec4..77c8351e3de7b 100644 --- a/stdlib/public/runtime/Bincompat.cpp +++ b/stdlib/public/runtime/Bincompat.cpp @@ -255,6 +255,19 @@ bool useLegacySwiftObjCHashing() { #endif } +// Controls legacy mode for the 'swift_task_isCurrentExecutorImpl' runtime function. +// +// In "legacy" / "no crash" mode: +// * The `swift_task_isCurrentExecutorImpl` cannot crash +// * This means cases where no "current" executor is present cannot be diagnosed correctly +// * The runtime can NOT use 'SerialExecutor/checkIsolated' +// * The runtime can NOT use 'dispatch_precondition' which is able ot handle some dispatch and main actor edge cases +// +// New behavior in "swift6" "crash" mode: +// * The 'swift_task_isCurrentExecutorImpl' will CRASH rather than return 'false' +// * This allows the method to invoke 'SerialExecutor/checkIsolated' +// * Which is allowed to call 'dispatch_precondition' and handle "on dispatch queue but not on Swift executor" cases +// // FIXME(concurrency): Once the release is announced, adjust the logic detecting the SDKs bool swift_bincompat_useLegacyNonCrashingExecutorChecks() { #if BINARY_COMPATIBILITY_APPLE diff --git a/stdlib/public/runtime/EnvironmentVariables.def b/stdlib/public/runtime/EnvironmentVariables.def index cb70e6702b2ce..3fb8e77ac46bf 100644 --- a/stdlib/public/runtime/EnvironmentVariables.def +++ b/stdlib/public/runtime/EnvironmentVariables.def @@ -121,7 +121,7 @@ VARIABLE(SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE, string, "", "non-crashing behavior. This flag enables temporarily restoring the " "legacy 'nocrash' behavior until adopting code has been adjusted. " "Legal values are: " - " 'nocrash' (Legacy behavior), " - " 'crash' (Swift 6.0+ behavior)") + " 'legacy' (Legacy behavior), " + " 'swift6' (Swift 6.0+ behavior)") #undef VARIABLE diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat_crash_swift_6_mode.swift similarity index 61% rename from test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat.swift rename to test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat_crash_swift_6_mode.swift index 97ffa075605d9..b29e3816151af 100644 --- a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat.swift +++ b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat_crash_swift_6_mode.swift @@ -1,4 +1,7 @@ -// RUN: %target-run-simple-swift(-parse-as-library -Xfrontend -disable-availability-checking) | %FileCheck %s +// RUN: %empty-directory(%t) +// RUN: %target-build-swift -Xfrontend -disable-availability-checking %import-libdispatch -parse-as-library %s -o %t/a.out +// RUN: %target-codesign %t/a.out +// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 %target-run %t/a.out // REQUIRES: executable_test // REQUIRES: concurrency @@ -11,6 +14,8 @@ // UNSUPPORTED: use_os_stdlib // UNSUPPORTED: freestanding +import StdlibUnittest + final class NaiveQueueExecutor: SerialExecutor { init() {} @@ -38,32 +43,26 @@ actor ActorOnNaiveQueueExecutor { self.executor.asUnownedSerialExecutor() } + // Executes on global pool, but our `checkIsolated` impl pretends + // that it is the same executor by never crashing. nonisolated func checkPreconditionIsolated() async { print("Before preconditionIsolated") self.preconditionIsolated() print("After preconditionIsolated") - - print("Before assumeIsolated") - self.assumeIsolated { iso in - print("Inside assumeIsolated") - } - print("After assumeIsolated") } } @main struct Main { static func main() async { - if #available(SwiftStdlib 6.0, *) { - let actor = ActorOnNaiveQueueExecutor() - await actor.checkPreconditionIsolated() - // CHECK: Before preconditionIsolated - // CHECK-NEXT: checkIsolated: pretend it is ok! - // CHECK-NEXT: After preconditionIsolated + let tests = TestSuite("AssertPreconditionIsolationTests") - // CHECK-NEXT: Before assumeIsolated - // CHECK-NEXT: checkIsolated: pretend it is ok! - // CHECK-NEXT: Inside assumeIsolated - // CHECK-NEXT: After assumeIsolated + if #available(SwiftStdlib 6.0, *) { + tests.test("[swift6+checkIsolated] Isolation assured by invoking 'checkIsolated'") { + let actor = ActorOnNaiveQueueExecutor() + await actor.checkPreconditionIsolated() + } } + + await runAllTestsAsync() } } diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat_nocrash_legacy_mode.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat_nocrash_legacy_mode.swift new file mode 100644 index 0000000000000..401eb2968bbaf --- /dev/null +++ b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat_nocrash_legacy_mode.swift @@ -0,0 +1,70 @@ +// RUN: %empty-directory(%t) +// RUN: %target-build-swift -Xfrontend -disable-availability-checking %import-libdispatch -parse-as-library %s -o %t/a.out +// RUN: %target-codesign %t/a.out +// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %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 + +final class NaiveQueueExecutor: SerialExecutor { + init() {} + + public func enqueue(_ job: consuming ExecutorJob) { + job.runSynchronously(on: self.asUnownedSerialExecutor()) + } + + public func asUnownedSerialExecutor() -> UnownedSerialExecutor { + UnownedSerialExecutor(ordinary: self) + } + + func checkIsolated() { + print("checkIsolated: pretend it is ok!") + } +} + +actor ActorOnNaiveQueueExecutor { + let executor: NaiveQueueExecutor + + init() { + self.executor = NaiveQueueExecutor() + } + + nonisolated var unownedExecutor: UnownedSerialExecutor { + self.executor.asUnownedSerialExecutor() + } + + // Executes on global pool, but our `checkIsolated` impl pretends + // that it is the same executor by never crashing. + nonisolated func checkPreconditionIsolated() async { + print("Before preconditionIsolated") + self.preconditionIsolated() + print("After preconditionIsolated") + } +} + +@main struct Main { + static func main() async { + let tests = TestSuite("AssertPreconditionIsolationTests") + + if #available(SwiftStdlib 6.0, *) { + tests.test("[legacy mode] expect crash since unable to invoke 'checkIsolated'") { + expectCrashLater() // In legacy mode we do NOT invoke 'checkIsolated' and therefore will crash + + let actor = ActorOnNaiveQueueExecutor() + await actor.checkPreconditionIsolated() + } + } + + await runAllTestsAsync() + } +} diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch_dispatchMain.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch_dispatchMain_swift_6_mode.swift similarity index 74% rename from test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch_dispatchMain.swift rename to test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch_dispatchMain_swift_6_mode.swift index 71412df080416..bfaa1d05467f5 100644 --- a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch_dispatchMain.swift +++ b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch_dispatchMain_swift_6_mode.swift @@ -1,7 +1,7 @@ // RUN: %empty-directory(%t) // RUN: %target-build-swift -Xfrontend -disable-availability-checking %import-libdispatch -parse-as-library %s -o %t/a.out // RUN: %target-codesign %t/a.out -// RUN: %target-run %t/a.out +// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 %target-run %t/a.out // REQUIRES: executable_test // REQUIRES: concurrency @@ -33,6 +33,10 @@ import Musl Swift.print("DispatchQueue.main.async { MainActor.precondition }") DispatchQueue.main.async { + // In Swift 6 mode we're allowed to notice that we're asking for main + // queue and use dispatch's assertions directly. + // + // The same code would be crashing without swift6 mode where we're allowed to use 'checkIsolated' MainActor.preconditionIsolated("I know I'm on the main queue") Swift.print("OK") diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch_swift6_mode.swift similarity index 97% rename from test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch.swift rename to test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch_swift6_mode.swift index f0f4f94a330e6..49503c13a8206 100644 --- a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch.swift +++ b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch_swift6_mode.swift @@ -1,7 +1,7 @@ // RUN: %empty-directory(%t) // RUN: %target-build-swift -Xfrontend -disable-availability-checking %import-libdispatch -parse-as-library %s -o %t/a.out // RUN: %target-codesign %t/a.out -// RUN: %target-run %t/a.out +// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 %target-run %t/a.out // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_main_customMain_swift6_mode.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_main_customMain_swift6_mode.swift new file mode 100644 index 0000000000000..6c6c91b3e1c03 --- /dev/null +++ b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_main_customMain_swift6_mode.swift @@ -0,0 +1,72 @@ +// RUN: %empty-directory(%t) +// RUN: %target-build-swift -Xfrontend -disable-availability-checking %import-libdispatch -parse-as-library %s -o %t/a.out +// RUN: %target-codesign %t/a.out +// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 %target-run %t/a.out +// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy not --crash %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 Dispatch + +@available(SwiftStdlib 6.0, *) +final class NaiveOnMainQueueExecutor: SerialExecutor { + init() {} + + let mainQueue = DispatchQueue.main + + public func enqueue(_ _job: consuming ExecutorJob) { + let job = UnownedJob(_job) + mainQueue.async { + job.runSynchronously(on: self.asUnownedSerialExecutor()) + } + } + + public func asUnownedSerialExecutor() -> UnownedSerialExecutor { + UnownedSerialExecutor(complexEquality: self) + } + + public func checkIsolated() { + print("\(Self.self).checkIsolated...") + dispatchPrecondition(condition: .onQueue(self.mainQueue)) + } +} + +actor ActorOnNaiveOnMainQueueExecutor { + let executor: NaiveOnMainQueueExecutor + + init() { + self.executor = NaiveOnMainQueueExecutor() + } + + nonisolated var unownedExecutor: UnownedSerialExecutor { + self.executor.asUnownedSerialExecutor() + } + + @MainActor + func checkPreconditionIsolated() async { + print("Before preconditionIsolated") + self.preconditionIsolated() + print("After preconditionIsolated") + } +} + +@main struct Main { + static func main() async { + if #available(SwiftStdlib 6.0, *) { + let actor = ActorOnNaiveOnMainQueueExecutor() + await actor.checkPreconditionIsolated() + // CHECK: Before preconditionIsolated + // CHECK-NEXT: checkIsolated: pretend it is ok! + // CHECK-NEXT: After preconditionIsolated + } + } +} diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_swift6_mode.swift similarity index 86% rename from test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated.swift rename to test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_swift6_mode.swift index 97ffa075605d9..9d54adca1994a 100644 --- a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated.swift +++ b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_swift6_mode.swift @@ -1,4 +1,7 @@ -// RUN: %target-run-simple-swift(-parse-as-library -Xfrontend -disable-availability-checking) | %FileCheck %s +// RUN: %empty-directory(%t) +// RUN: %target-build-swift -Xfrontend -disable-availability-checking %import-libdispatch -parse-as-library %s -o %t/a.out +// RUN: %target-codesign %t/a.out +// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 %target-run %t/a.out // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor_default_mode.swift similarity index 99% rename from test/Concurrency/Runtime/actor_assert_precondition_executor.swift rename to test/Concurrency/Runtime/actor_assert_precondition_executor_default_mode.swift index 98827587b76b3..1e47231de33bb 100644 --- a/test/Concurrency/Runtime/actor_assert_precondition_executor.swift +++ b/test/Concurrency/Runtime/actor_assert_precondition_executor_default_mode.swift @@ -1,7 +1,7 @@ // 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 +// RUN: %target-run %t/a.out // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Interpreter/preconcurrency_conformances.swift b/test/Interpreter/preconcurrency_conformances.swift index efe7c2e18f045..c45872a98a14d 100644 --- a/test/Interpreter/preconcurrency_conformances.swift +++ b/test/Interpreter/preconcurrency_conformances.swift @@ -16,19 +16,23 @@ // RUN: %target-build-swift -Xfrontend -enable-upcoming-feature -Xfrontend DynamicActorIsolation -I %t -L %t -l Types %t/src/Crash1.swift -o %t/crash1.out // RUN: %target-codesign %t/crash1.out -// RUN: not --crash env SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash1.out 2>&1 | %FileCheck %t/src/Crash1.swift +// RUN: not --crash env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash1.out 2>&1 | %FileCheck %t/src/Crash1.swift --check-prefix=LEGACY_CHECK +// RUN: not --crash env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash1.out 2>&1 | %FileCheck %t/src/Crash1.swift --check-prefix=SWIFT6_CHECK --dump-input=always // RUN: %target-build-swift -Xfrontend -enable-upcoming-feature -Xfrontend DynamicActorIsolation -I %t -L %t -l Types %t/src/Crash2.swift -o %t/crash2.out // RUN: %target-codesign %t/crash2.out -// RUN: not --crash env SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash2.out 2>&1 | %FileCheck %t/src/Crash2.swift +// RUN: not --crash env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash2.out 2>&1 | %FileCheck %t/src/Crash2.swift --check-prefix=LEGACY_CHECK +// RUN: not --crash env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash2.out 2>&1 | %FileCheck %t/src/Crash2.swift --check-prefix=SWIFT6_CHECK --dump-input=always // RUN: %target-build-swift -Xfrontend -enable-upcoming-feature -Xfrontend DynamicActorIsolation -I %t -L %t -l Types %t/src/Crash3.swift -o %t/crash3.out // RUN: %target-codesign %t/crash3.out -// RUN: not --crash env SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash3.out 2>&1 | %FileCheck %t/src/Crash3.swift +// RUN: not --crash env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash3.out 2>&1 | %FileCheck %t/src/Crash3.swift --check-prefix=LEGACY_CHECK +// RUN: not --crash env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash3.out 2>&1 | %FileCheck %t/src/Crash3.swift --check-prefix=SWIFT6_CHECK --dump-input=always // RUN: %target-build-swift -Xfrontend -enable-upcoming-feature -Xfrontend DynamicActorIsolation -I %t -L %t -l Types %t/src/Crash4.swift -o %t/crash4.out // RUN: %target-codesign %t/crash4.out -// RUN: not --crash env SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash4.out 2>&1 | %FileCheck %t/src/Crash4.swift +// RUN: not --crash env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash4.out 2>&1 | %FileCheck %t/src/Crash4.swift --check-prefix=LEGACY_CHECK +// RUN: not --crash env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/crash4.out 2>&1 | %FileCheck %t/src/Crash4.swift --check-prefix=SWIFT6_CHECK --dump-input=always // REQUIRES: asserts // REQUIRES: concurrency @@ -86,19 +90,35 @@ extension ActorTest : @preconcurrency P { //--- Crash1.swift import Types print(await runTest(Test.self)) -// CHECK: Incorrect actor executor assumption; Expected MainActor executor +print("OK") +// LEGACY_CHECK: @MainActor function at Types/Types.swift:16 was not called on the main thread + +// Crash without good message, since via 'dispatch_assert_queue' +// SWIFT6_CHECK-NOT: OK //--- Crash2.swift import Types print(await runAccessors(Test.self)) -// CHECK: Incorrect actor executor assumption; Expected MainActor executor +print("OK") +// LEGACY_CHECK: data race detected: @MainActor function at Types/Types.swift:15 was not called on the main thread + +// Crash without good message, since via 'dispatch_assert_queue' +// SWIFT6_CHECK-NOT: OK //--- Crash3.swift import Types print(await runTest(ActorTest.self)) -// CHECK: Incorrect actor executor assumption +print("OK") +// LEGACY_CHECK: data race detected: actor-isolated function at Types/Types.swift:33 was not called on the same actor + +// SWIFT6_CHECK: Incorrect actor executor assumption +// SWIFT6_CHECK-NOT: OK //--- Crash4.swift import Types print(await runAccessors(ActorTest.self)) -// CHECK: Incorrect actor executor assumption +print("OK") +// LEGACY_CHECK: data race detected: actor-isolated function at Types/Types.swift:30 was not called on the same actor + +// SWIFT6_CHECK: Incorrect actor executor assumption +// SWIFT6_CHECK-NOT: OK diff --git a/test/Interpreter/preconcurrency_conformances_with_disabled_checks.swift b/test/Interpreter/preconcurrency_conformances_with_disabled_checks.swift index 911daa27a22de..c681e65748627 100644 --- a/test/Interpreter/preconcurrency_conformances_with_disabled_checks.swift +++ b/test/Interpreter/preconcurrency_conformances_with_disabled_checks.swift @@ -18,18 +18,26 @@ // RUN: %target-build-swift -I %t -L %t -l Types %t/src/Test1.swift -o %t/test1.out // RUN: %target-codesign %t/test1.out // RUN: env SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test1.out 2>&1 | %FileCheck %t/src/Test1.swift +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test1.out 2>&1 | %FileCheck %t/src/Test1.swift +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test1.out 2>&1 | %FileCheck %t/src/Test1.swift // RUN: %target-build-swift -I %t -L %t -l Types %t/src/Test2.swift -o %t/test2.out // RUN: %target-codesign %t/test2.out // RUN: env SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test2.out 2>&1 | %FileCheck %t/src/Test2.swift +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test2.out 2>&1 | %FileCheck %t/src/Test2.swift +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test2.out 2>&1 | %FileCheck %t/src/Test2.swift // RUN: %target-build-swift -I %t -L %t -l Types %t/src/Test3.swift -o %t/test3.out // RUN: %target-codesign %t/test3.out // RUN: env SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test3.out 2>&1 | %FileCheck %t/src/Test3.swift +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test3.out 2>&1 | %FileCheck %t/src/Test3.swift +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test3.out 2>&1 | %FileCheck %t/src/Test3.swift // RUN: %target-build-swift -I %t -L %t -l Types %t/src/Test4.swift -o %t/test4.out // RUN: %target-codesign %t/test4.out // RUN: env SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test4.out 2>&1 | %FileCheck %t/src/Test4.swift +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test4.out 2>&1 | %FileCheck %t/src/Test4.swift +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=2 %target-run %t/test4.out 2>&1 | %FileCheck %t/src/Test4.swift // REQUIRES: asserts // REQUIRES: concurrency diff --git a/test/Interpreter/preconcurrency_conformances_with_legacy_isSameExecutor.swift b/test/Interpreter/preconcurrency_conformances_with_legacy_isSameExecutor.swift new file mode 100644 index 0000000000000..927ccc79a269d --- /dev/null +++ b/test/Interpreter/preconcurrency_conformances_with_legacy_isSameExecutor.swift @@ -0,0 +1,105 @@ +// RUN: %empty-directory(%t/src) +// RUN: split-file %s %t/src + +// RUN: %target-build-swift %t/src/Interface.swift -emit-module -emit-library \ +// RUN: -target %target-cpu-apple-macosx10.15 -swift-version 5 \ +// RUN: -enable-library-evolution \ +// RUN: -module-name Interface \ +// RUN: -o %t/%target-library-name(Interface) \ +// RUN: -emit-module-interface-path %t/Interface.swiftinterface + +// RUN: %target-build-swift %t/src/Types.swift -swift-version 5 -emit-module -emit-library -enable-library-evolution -module-name Types -o %t/%target-library-name(Types) \ +// RUN: -target %target-cpu-apple-macosx10.15 \ +// RUN: -I %t -L %t -l Interface \ +// RUN: -emit-module-interface-path %t/Types.swiftinterface \ +// RUN: -Xfrontend -enable-upcoming-feature -Xfrontend DynamicActorIsolation \ +// RUN: -Xfrontend -disable-dynamic-actor-isolation + +// RUN: %target-build-swift -I %t -L %t -l Types %t/src/Test1.swift -o %t/test1.out +// RUN: %target-codesign %t/test1.out +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %target-run %t/test1.out 2>&1 | %FileCheck %t/src/Test1.swift + +// RUN: %target-build-swift -I %t -L %t -l Types %t/src/Test2.swift -o %t/test2.out +// RUN: %target-codesign %t/test2.out +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %target-run %t/test2.out 2>&1 | %FileCheck %t/src/Test2.swift + +// RUN: %target-build-swift -I %t -L %t -l Types %t/src/Test3.swift -o %t/test3.out +// RUN: %target-codesign %t/test3.out +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %target-run %t/test3.out 2>&1 | %FileCheck %t/src/Test3.swift + +// RUN: %target-build-swift -I %t -L %t -l Types %t/src/Test4.swift -o %t/test4.out +// RUN: %target-codesign %t/test4.out +// RUN: env SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %target-run %t/test4.out 2>&1 | %FileCheck %t/src/Test4.swift + +// REQUIRES: asserts +// REQUIRES: concurrency +// REQUIRES: concurrency_runtime +// REQUIRES: executable_test +// REQUIRES: OS=macosx + +// rdar://123810657 +// UNSUPPORTED: back_deployment_runtime + +//--- Interface.swift +public protocol P { + init() + + var prop: [String] { get set } + func test() -> Int +} + +//--- Types.swift +import Interface + +public func runTest(_ type: T.Type) async -> Int { + let v = type.init() + return v.test() +} + +public func runAccessors(_ type: T.Type) async -> [String] { + var v = type.init() + v.prop = ["a", "b", "c"] + return v.prop +} + +public final class Test : @preconcurrency P { + @MainActor public var prop: [String] = [] + @MainActor public func test() -> Int { 42 } + + public init() {} +} + +public actor ActorTest { + var x: Int = 0 + + public init() {} +} + +extension ActorTest : @preconcurrency P { + public var prop: [String] { + get { [] } + set { } + } + + public func test() -> Int { x } +} + +//--- Test1.swift +import Types +print(await runTest(Test.self)) +// CHECK-NOT: Incorrect actor executor assumption; Expected MainActor executor + +//--- Test2.swift +import Types +print(await runAccessors(Test.self)) +// CHECK-NOT: Incorrect actor executor assumption; Expected MainActor executor + +//--- Test3.swift +import Types +print(await runTest(ActorTest.self)) +// CHECK-NOT: Incorrect actor executor assumption + +//--- Test4.swift +import Types +print(await runAccessors(ActorTest.self)) +// CHECK-NOT: Incorrect actor executor assumption From d36f99540ca3267e41cc73766c8bbaac089bd13d Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Wed, 22 May 2024 22:02:22 +0900 Subject: [PATCH 3/8] [Concurrency] Always assume that if expecting main actor and are on main thread that this is equal --- stdlib/public/Concurrency/Actor.cpp | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/stdlib/public/Concurrency/Actor.cpp b/stdlib/public/Concurrency/Actor.cpp index e1558da08a6ff..45e1952437d76 100644 --- a/stdlib/public/Concurrency/Actor.cpp +++ b/stdlib/public/Concurrency/Actor.cpp @@ -387,17 +387,9 @@ static bool swift_task_isCurrentExecutorImpl(SerialExecutorRef expectedExecutor) // the expected executor however, so we need to try a bit harder before // we fail. - // Legacy special handling the main executor by detecting the main thread. - // - // When 'checkIsolated' is available it will perform a dispatch queue assertion - // against the main queue, potentially resulting in a crash (expected). - // - // In legacy mode, we cannot allow crashes here, and therefore we keep the - // special best-effort handling of the "main thread". - if (isCurrentExecutorMode == Legacy_NoCheckIsolated_NonCrashing) { - if (expectedExecutor.isMainExecutor() && isExecutingOnMainThread()) { - return true; - } + // Special handling the main executor by detecting the main thread. + if (expectedExecutor.isMainExecutor() && isExecutingOnMainThread()) { + return true; } // We cannot use 'complexEquality' as it requires two executor instances, From 153806b71a8351fd154dac89d8bbdc7969e9b2e9 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Wed, 22 May 2024 22:32:36 +0900 Subject: [PATCH 4/8] [Concurrency] Reshape initial values of executor checking so tests pass on Linux --- stdlib/public/Concurrency/Actor.cpp | 32 +++++++++++-------- ... actor_assert_precondition_executor.swift} | 4 +-- ...in_customExecutorOnMain_swift6_mode.swift} | 0 .../Runtime/actor_assume_executor.swift | 2 +- .../data_race_detection_legacy_warning.swift | 6 ++-- 5 files changed, 24 insertions(+), 20 deletions(-) rename test/Concurrency/Runtime/{actor_assert_precondition_executor_default_mode.swift => actor_assert_precondition_executor.swift} (95%) rename test/Concurrency/Runtime/{actor_assert_precondition_executor_checkIsolated_main_customMain_swift6_mode.swift => actor_assert_precondition_executor_checkIsolated_main_customExecutorOnMain_swift6_mode.swift} (100%) diff --git a/stdlib/public/Concurrency/Actor.cpp b/stdlib/public/Concurrency/Actor.cpp index 45e1952437d76..f350fc2566e7b 100644 --- a/stdlib/public/Concurrency/Actor.cpp +++ b/stdlib/public/Concurrency/Actor.cpp @@ -334,10 +334,9 @@ enum IsCurrentExecutorCheckMode: unsigned { static IsCurrentExecutorCheckMode isCurrentExecutorMode = Swift6_UseCheckIsolated_AllowCrash; - // Shimming call to Swift runtime because Swift Embedded does not have // these symbols defined. -bool swift_bincompat_useLegacyNonCrashingExecutorChecks() { +bool __swift_bincompat_useLegacyNonCrashingExecutorChecks() { #if !SWIFT_CONCURRENCY_EMBEDDED return swift::runtime::bincompat:: swift_bincompat_useLegacyNonCrashingExecutorChecks(); @@ -346,22 +345,30 @@ bool swift_bincompat_useLegacyNonCrashingExecutorChecks() { #endif } -// Check override of executor checking mode. -static void checkIsCurrentExecutorMode(void *context) { - auto useLegacyMode = - swift_bincompat_useLegacyNonCrashingExecutorChecks(); +// Done this way because of the interaction with the initial value of +// 'unexpectedExecutorLogLevel' +bool swift_bincompat_useLegacyNonCrashingExecutorChecks() { + bool legacyMode = __swift_bincompat_useLegacyNonCrashingExecutorChecks(); // Potentially, override the platform detected mode, primarily used in tests. #if SWIFT_STDLIB_HAS_ENVIRON - if (const char *modeStr = runtime::environment::concurrencyIsCurrentExecutorLegacyModeOverride()) { + if (const char *modeStr = runtime::environment:: + concurrencyIsCurrentExecutorLegacyModeOverride()) { if (strcmp(modeStr, "nocrash") == 0 || strcmp(modeStr, "legacy") == 0) { - useLegacyMode = true; + return true; } else if (strcmp(modeStr, "crash") == 0 || strcmp(modeStr, "swift6") == 0) { - useLegacyMode = false; + return false; // don't use the legacy mode } // else, just use the platform detected mode - } + } // no override, use the default mode #endif // SWIFT_STDLIB_HAS_ENVIRON + return legacyMode; +} + +// Check override of executor checking mode. +static void checkIsCurrentExecutorMode(void *context) { + bool useLegacyMode = + swift_bincompat_useLegacyNonCrashingExecutorChecks(); isCurrentExecutorMode = useLegacyMode ? Legacy_NoCheckIsolated_NonCrashing : Swift6_UseCheckIsolated_AllowCrash; } @@ -520,12 +527,9 @@ static void checkUnexpectedExecutorLogLevel(void *context) { if (!levelStr) return; - auto isCurrentExecutorLegacyMode = - swift_bincompat_useLegacyNonCrashingExecutorChecks(); - long level = strtol(levelStr, nullptr, 0); if (level >= 0 && level < 3) { - if (isCurrentExecutorLegacyMode) { + if (swift_bincompat_useLegacyNonCrashingExecutorChecks()) { // legacy mode permits doing nothing or just logging, since the method // used to perform the check itself is not going to crash: unexpectedExecutorLogLevel = level; diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor_default_mode.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor.swift similarity index 95% rename from test/Concurrency/Runtime/actor_assert_precondition_executor_default_mode.swift rename to test/Concurrency/Runtime/actor_assert_precondition_executor.swift index 1e47231de33bb..a0fd45909a76e 100644 --- a/test/Concurrency/Runtime/actor_assert_precondition_executor_default_mode.swift +++ b/test/Concurrency/Runtime/actor_assert_precondition_executor.swift @@ -1,7 +1,7 @@ // 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 +// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %target-run %t/a.out // REQUIRES: executable_test // REQUIRES: concurrency @@ -12,7 +12,7 @@ // UNSUPPORTED: use_os_stdlib // UNSUPPORTED: freestanding -// rdar://119743909 fails in optimze tests. +// rdar://119743909 fails in optimize tests. // UNSUPPORTED: swift_test_mode_optimize // UNSUPPORTED: swift_test_mode_optimize_size diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_main_customMain_swift6_mode.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_main_customExecutorOnMain_swift6_mode.swift similarity index 100% rename from test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_main_customMain_swift6_mode.swift rename to test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_main_customExecutorOnMain_swift6_mode.swift diff --git a/test/Concurrency/Runtime/actor_assume_executor.swift b/test/Concurrency/Runtime/actor_assume_executor.swift index c178b3318a3a2..5d33ae969988f 100644 --- a/test/Concurrency/Runtime/actor_assume_executor.swift +++ b/test/Concurrency/Runtime/actor_assume_executor.swift @@ -1,7 +1,7 @@ // 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 +// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %target-run %t/a.out // REQUIRES: executable_test // REQUIRES: concurrency diff --git a/test/Concurrency/Runtime/data_race_detection_legacy_warning.swift b/test/Concurrency/Runtime/data_race_detection_legacy_warning.swift index ec1bce9a98445..dc891057d22f2 100644 --- a/test/Concurrency/Runtime/data_race_detection_legacy_warning.swift +++ b/test/Concurrency/Runtime/data_race_detection_legacy_warning.swift @@ -6,7 +6,7 @@ // will be able to have this behavior, however new apps will not. We use the // overrides to test the logic for legacy code remains functional. // -// RUN: env %env-SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=1 %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=nocrash %target-run %t/a.out 2>&1 | %FileCheck %s +// RUN: %env-SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL=1 %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %target-run %t/a.out 2>&1 | %FileCheck %s // REQUIRES: executable_test // REQUIRES: concurrency @@ -66,14 +66,14 @@ actor MyActor { struct Runner { static func main() async { print("Launching a main-actor task") - // CHECK: warning: data race detected: @MainActor function at main/data_race_detection_legacy_warning.swift:30 was not called on the main thread + // CHECK: data race detected: @MainActor function at main/data_race_detection_legacy_warning.swift:30 was not called on the main thread launchFromMainThread() sleep(1) let actor = MyActor() let actorFn = await actor.getTaskOnMyActor() print("Launching an actor-instance task") - // CHECK: warning: data race detected: actor-isolated function at main/data_race_detection_legacy_warning.swift:59 was not called on the same actor + // CHECK: data race detected: actor-isolated function at main/data_race_detection_legacy_warning.swift:59 was not called on the same actor launchTask(actorFn) sleep(1) From 80ce40b3bbe7c5e9738a2b5a0ea81934e89726f0 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Thu, 23 May 2024 16:51:32 +0900 Subject: [PATCH 5/8] [Concurrency] adjust custom_executors_complex_equality_crash.swift for swift6/legacy modes --- ...ustom_executors_complex_equality_crash.swift | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift b/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift index 2720c104a9582..306e631ccbed4 100644 --- a/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift +++ b/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift @@ -1,4 +1,11 @@ -// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking %import-libdispatch -parse-as-library) +// NOT: %target-run-simple-swift( -Xfrontend -disable-availability-checking %import-libdispatch -parse-as-library) + +// 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: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %target-run %import-libdispatch %t/a.out +// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 %target-run %import-libdispatch %t/a.out + // REQUIRES: concurrency // REQUIRES: executable_test @@ -7,7 +14,7 @@ // rdar://106849189 move-only types should be supported in freestanding mode // UNSUPPORTED: freestanding -// rdar://119743909 fails in optimze tests. +// rdar://119743909 fails in optimized tests. // UNSUPPORTED: swift_test_mode_optimize // UNSUPPORTED: swift_test_mode_optimize_size @@ -86,9 +93,9 @@ struct Runner { await actor.test(expectedExecutor: two) } tests.test("isSameExclusiveContext=false, causes same executor checks to crash") { - expectCrashLater(withMessage: "Precondition failed: Incorrect actor executor assumption; " + - "Expected 'NaiveQueueExecutor(unknown)' executor. " + - "Expected deep equality to trigger for ") + // In Swift6 mode the error message for the crash depends on dispatch, so it is lower + // quality than our specialized messages; We cannot assert on the text since we run in both modes. + expectCrashLater() let unknown = NaiveQueueExecutor(name: "unknown", DispatchQueue(label: "unknown")) let actor = MyActor(executor: one) From 17937e94e8aa7bdad840d91abb4e28979e218905 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Mon, 27 May 2024 11:06:29 +0900 Subject: [PATCH 6/8] fix missing %import-libdispatch in test --- .../Runtime/custom_executors_complex_equality_crash.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift b/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift index 306e631ccbed4..9b1410ff8e223 100644 --- a/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift +++ b/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift @@ -1,7 +1,7 @@ // NOT: %target-run-simple-swift( -Xfrontend -disable-availability-checking %import-libdispatch -parse-as-library) // RUN: %empty-directory(%t) -// RUN: %target-build-swift -Xfrontend -disable-availability-checking -parse-as-library %s -o %t/a.out +// RUN: %target-build-swift -Xfrontend -disable-availability-checking %import-libdispatch -parse-as-library %s -o %t/a.out // RUN: %target-codesign %t/a.out // RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %target-run %import-libdispatch %t/a.out // RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 %target-run %import-libdispatch %t/a.out From b90aad2244d50008bc4f379c88267e0bf36b436a Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Mon, 27 May 2024 17:01:19 +0900 Subject: [PATCH 7/8] [Concurrency] add missing requires libdispatch, unbreaks Windows test --- .../Runtime/actor_assert_precondition_executor.swift | 3 +++ test/Concurrency/Runtime/actor_assume_executor.swift | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/test/Concurrency/Runtime/actor_assert_precondition_executor.swift b/test/Concurrency/Runtime/actor_assert_precondition_executor.swift index a0fd45909a76e..8c2ec812b9ea2 100644 --- a/test/Concurrency/Runtime/actor_assert_precondition_executor.swift +++ b/test/Concurrency/Runtime/actor_assert_precondition_executor.swift @@ -7,6 +7,9 @@ // REQUIRES: concurrency // REQUIRES: concurrency_runtime +// TODO: The actual reason is that we do these %env- tricks, which e.g. Windows is confused about +// REQUIRES: libdispatch + // UNSUPPORTED: back_deployment_runtime // UNSUPPORTED: back_deploy_concurrency // UNSUPPORTED: use_os_stdlib diff --git a/test/Concurrency/Runtime/actor_assume_executor.swift b/test/Concurrency/Runtime/actor_assume_executor.swift index 5d33ae969988f..50048d101e4b9 100644 --- a/test/Concurrency/Runtime/actor_assume_executor.swift +++ b/test/Concurrency/Runtime/actor_assume_executor.swift @@ -6,7 +6,9 @@ // REQUIRES: executable_test // REQUIRES: concurrency // REQUIRES: concurrency_runtime -// UNSUPPORTED: back_deployment_runtime + +// TODO: The actual reason is that we do these %env- tricks, which e.g. Windows is confused about +// REQUIRES: libdispatch // UNSUPPORTED: back_deploy_concurrency // UNSUPPORTED: use_os_stdlib From 347864b598cc92ab6ae634a969f0611ced365d92 Mon Sep 17 00:00:00 2001 From: Konrad `ktoso` Malawski Date: Tue, 28 May 2024 12:05:56 +0900 Subject: [PATCH 8/8] disable custom_executors_complex_equality_crash on linux for now --- .../Runtime/custom_executors_complex_equality_crash.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift b/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift index 9b1410ff8e223..4290618008890 100644 --- a/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift +++ b/test/Concurrency/Runtime/custom_executors_complex_equality_crash.swift @@ -6,6 +6,8 @@ // RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=legacy %target-run %import-libdispatch %t/a.out // RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 %target-run %import-libdispatch %t/a.out +// TODO: Need to find out how to combine %env- and %target-run and %import-libdispatch reliably. +// UNSUPPORTED: OS=linux-gnu // REQUIRES: concurrency // REQUIRES: executable_test