[Concurrency] Further test cleanups, making sure modes interop properly

Author: ktoso

stdlib/public/Concurrency/Actor.cpp

@@ -337,13 +337,14 @@ static IsCurrentExecutorCheckMode isCurrentExecutorMode =
 
 
 // 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.
@@ -353,17 +354,15 @@ 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;
 }
@@ -425,32 +424,6 @@ 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.
-    if (isCurrentExecutorMode == Default_UseCheckIsolated_AllowCrash) {
-      swift_Concurrency_fatalError(0, "Incorrect actor executor assumption");
-    }
-    return false;
-  }
-
-  if (expectedExecutor.isMainExecutor() && !currentExecutor.isMainExecutor()) {
-    if (isCurrentExecutorMode == Default_UseCheckIsolated_AllowCrash) {
-      // TODO: Invoke checkIsolated() on "main" SerialQueue once it implements `checkIsolated`, for potentially better/more consistent error messages
-      swift_Concurrency_fatalError(0, "Incorrect actor executor assumption; Expected MainActor executor");
-    }
-    return false;
-  } else if (!expectedExecutor.isMainExecutor() && currentExecutor.isMainExecutor()) {
-    if (isCurrentExecutorMode == Default_UseCheckIsolated_AllowCrash) {
-      // TODO: Invoke checkIsolated() on "main" SerialQueue once it implements `checkIsolated`, for potentially better/more consistent error messages
-      swift_Concurrency_fatalError(0, "Incorrect actor executor assumption; Expected not-MainActor executor");
-    }
-    return false;
-  }
-
   if (expectedExecutor.isComplexEquality()) {
     if (currentExecutor.getIdentity() && expectedExecutor.getIdentity() &&
         swift_compareWitnessTables(
@@ -470,7 +443,7 @@ static bool swift_task_isCurrentExecutorImpl(SerialExecutorRef expectedExecutor)
       // chance to check.
       if (isSameExclusiveExecutionContextResult) {
         return true;
-      }
+      } // else, we must give 'checkIsolated' a last chance to verify isolation
     }
   }
 
@@ -497,17 +470,17 @@ static bool swift_task_isCurrentExecutorImpl(SerialExecutorRef expectedExecutor)
   // compat_invoke_swift_task_checkIsolated(expectedExecutor);
   if (isCurrentExecutorMode == Default_UseCheckIsolated_AllowCrash) {
     swift_task_checkIsolated(expectedExecutor);
+
     // The checkIsolated call did not crash, so we are on the right executor.
     return true;
   }
 
-  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
@@ -524,9 +497,25 @@ 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. This means that it is not possible
+      // to support logging or warning 'unexpected executor log level' in this mode.
+      //
+      // Thus, the only possible mode to support is the 'fatal error' mode.
+      unexpectedExecutorLogLevel = 2;
+    }
+  }
 #endif // SWIFT_STDLIB_HAS_ENVIRON
 }
 

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

test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_bincompat.swift renamed to 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()
   }
 }

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()
+  }
+}

test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch_dispatchMain.swift renamed to 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")

test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated_dispatch.swift renamed to 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

test/Concurrency/Runtime/actor_assert_precondition_executor_checkIsolated.swift renamed to 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

test/Concurrency/Runtime/actor_assert_precondition_executor.swift renamed to 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

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