🍒 [5.7][Distributed] Dont hang calls on not found accessors swiftlang#60266 (swiftlang#60476)

Author: ktoso

stdlib/public/Distributed/DistributedActor.cpp

@@ -109,6 +109,10 @@ static void ::swift_distributed_execute_target_resume(
   return resumeInParent(parentCtx, error);
 }
 
+SWIFT_CC(swift) SWIFT_RUNTIME_STDLIB_INTERNAL
+SwiftError* swift_distributed_makeDistributedTargetAccessorNotFoundError(
+    const char *targetNameStart, size_t targetNameLength);
+
 SWIFT_CC(swiftasync)
 void ::swift_distributed_execute_target(
     SWIFT_ASYNC_CONTEXT AsyncContext *callerContext,
@@ -124,8 +128,12 @@ void ::swift_distributed_execute_target(
     void **decoderWitnessTable) {
   auto *accessor = findDistributedAccessor(targetNameStart, targetNameLength);
   if (!accessor) {
-    assert(false && "no distributed accessor");
-    return; // FIXME(distributed): return -1 here so the lib can fail the call
+    SwiftError *error =
+        swift_distributed_makeDistributedTargetAccessorNotFoundError(targetNameStart, targetNameLength);
+    auto resumeInParent =
+        reinterpret_cast<TargetExecutorSignature::ContinuationType *>(
+            callerContext->ResumeParent);
+    return resumeInParent(callerContext, error);
   }
 
   auto *asyncFnPtr = reinterpret_cast<

stdlib/public/Distributed/DistributedActorSystem.swift

@@ -207,6 +207,10 @@ extension DistributedActorSystem {
   ///           some other mismatch between them happens. In general, this
   ///           method is allowed to throw in any situation that might otherwise
   ///           result in an illegal or unexpected invocation being performed.
+  ///
+  ///           Throws ``ExecuteDistributedTargetMissingAccessorError`` if the `target`
+  ///           does not resolve to a valid distributed function accessor, i.e. the
+  ///           call identifier is incorrect, corrupted, or simply not present in this process.
   public func executeDistributedTarget<Act>(
     on actor: Act,
     target: RemoteCallTarget,
@@ -242,7 +246,8 @@ extension DistributedActorSystem {
       let subs = try invocationDecoder.decodeGenericSubstitutions()
       if subs.isEmpty {
         throw ExecuteDistributedTargetError(
-          message: "Cannot call generic method without generic argument substitutions")
+          message: "Cannot call generic method without generic argument substitutions",
+          errorCode: .missingGenericSubstitutions)
       }
 
       substitutionsBuffer = .allocate(capacity: subs.count)
@@ -256,7 +261,8 @@ extension DistributedActorSystem {
                                                                      genericArguments: substitutionsBuffer!)
       if numWitnessTables < 0 {
         throw ExecuteDistributedTargetError(
-          message: "Generic substitutions \(subs) do not satisfy generic requirements of \(target) (\(targetName))")
+          message: "Generic substitutions \(subs) do not satisfy generic requirements of \(target) (\(targetName))",
+          errorCode: .invalidGenericSubstitutions)
       }
     }
 
@@ -270,7 +276,8 @@ extension DistributedActorSystem {
                  Failed to decode distributed invocation target expected parameter count,
                  error code: \(paramCount)
                  mangled name: \(targetName)
-                 """)
+                 """,
+        errorCode: .invalidParameterCount)
     }
 
     // Prepare buffer for the parameter types to be decoded into:
@@ -295,7 +302,8 @@ extension DistributedActorSystem {
                  Failed to decode the expected number of params of distributed invocation target, error code: \(decodedNum)
                  (decoded: \(decodedNum), expected params: \(paramCount)
                  mangled name: \(targetName)
-                 """)
+                 """,
+        errorCode: .invalidParameterCount)
     }
 
     // Copy the types from the buffer into a Swift Array
@@ -316,12 +324,14 @@ extension DistributedActorSystem {
                                                                     genericEnv: genericEnv,
                                                                     genericArguments: substitutionsBuffer) else {
       throw ExecuteDistributedTargetError(
-        message: "Failed to decode distributed target return type")
+        message: "Failed to decode distributed target return type",
+        errorCode: .typeDeserializationFailure)
     }
 
     guard let resultBuffer = _openExistential(returnTypeFromTypeInfo, do: allocateReturnTypeBuffer) else {
       throw ExecuteDistributedTargetError(
-        message: "Failed to allocate buffer for distributed target return type")
+        message: "Failed to allocate buffer for distributed target return type",
+        errorCode: .typeDeserializationFailure)
     }
 
     func destroyReturnTypeBuffer<R>(_: R.Type) {
@@ -567,12 +577,49 @@ public protocol DistributedTargetInvocationResultHandler {
 @available(SwiftStdlib 5.7, *)
 public protocol DistributedActorSystemError: Error {}
 
+/// Error thrown by ``DistributedActorSystem/executeDistributedTarget(on:target:invocationDecoder:handler:)``.
+///
+/// Inspect the ``errorCode`` for details about the underlying reason this error was thrown.
 @available(SwiftStdlib 5.7, *)
 public struct ExecuteDistributedTargetError: DistributedActorSystemError {
-  let message: String
+  public let errorCode: ErrorCode
+  public let message: String
+
+  public enum ErrorCode {
+    /// Unable to resolve the target identifier to a function accessor.
+    /// This can happen when the identifier is corrupt, illegal, or wrong in the
+    /// sense that the caller and callee do not have the called function recorded
+    /// using the same identifier.
+    case targetAccessorNotFound
+
+    /// Call target has different number of parameters than arguments
+    /// provided by the invocation decoder.
+    case invalidParameterCount
+
+    /// Target expects generic environment information, but invocation decoder
+    /// provided no generic substitutions.
+    case missingGenericSubstitutions
+
+    /// Generic substitutions provided by invocation decoder are incompatible
+    /// with target of the call. E.g. the generic requirements on the actual
+    /// target could not be fulfilled by the obtained generic substitutions.
+    case invalidGenericSubstitutions
+
+    // Failed to deserialize type or obtain type information for call.
+    case typeDeserializationFailure
+
+    /// A general issue during the execution of the distributed call target occurred.
+    case other
+  }
 
   public init(message: String) {
     self.message = message
+    self.errorCode = .other
+  }
+
+  public init(message: String, errorCode: ErrorCode) {
+    self.message = message
+    self.errorCode = errorCode
   }
 }
 

stdlib/public/Distributed/DistributedMetadata.swift

@@ -108,3 +108,13 @@ func _getWitnessTablesFor(
   environment: UnsafeRawPointer,
   genericArguments: UnsafeRawPointer
 ) -> (UnsafeRawPointer, Int)
+
+@available(SwiftStdlib 5.7, *)
+@_silgen_name("swift_distributed_makeDistributedTargetAccessorNotFoundError")
+internal // SPI Distributed
+func _makeDistributedTargetAccessorNotFoundError() -> Error {
+  /// We don't include the name of the target in case the input was compromised.
+  return ExecuteDistributedTargetError(
+    message: "Failed to locate distributed function accessor",
+    errorCode: .targetAccessorNotFound)
+}

test/Distributed/Runtime/distributed_actor_func_calls_remoteCall_bad_targets.swift

@@ -0,0 +1,73 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule -module-name FakeDistributedActorSystems -disable-availability-checking %S/../Inputs/FakeDistributedActorSystems.swift
+// RUN: %target-build-swift -module-name main -Xfrontend -disable-availability-checking -j2 -parse-as-library -I %t %s %S/../Inputs/FakeDistributedActorSystems.swift -o %t/a.out
+// RUN: %target-run %t/a.out | %FileCheck %s --color
+
+// REQUIRES: executable_test
+// REQUIRES: concurrency
+// REQUIRES: distributed
+
+// rdar://76038845
+// UNSUPPORTED: use_os_stdlib
+// UNSUPPORTED: back_deployment_runtime
+
+// FIXME(distributed): Distributed actors currently have some issues on windows, isRemote always returns false. rdar://82593574
+// UNSUPPORTED: OS=windows-msvc
+
+import Distributed
+import FakeDistributedActorSystems
+
+typealias DefaultDistributedActorSystem = FakeRoundtripActorSystem
+
+distributed actor Greeter {
+  distributed func greet(name: String) -> String {
+      "Hello, \(name)!"
+  }
+}
+
+func test() async throws {
+  let system = DefaultDistributedActorSystem()
+  let local = Greeter(actorSystem: system)
+  let ref = try Greeter.resolve(id: local.id, using: system)
+
+  // Make sure normal call works ok:
+  let greeting = try await ref.greet(name: "Caplin")
+  print("\(greeting)")
+  // CHECK: Hello, Caplin!
+
+  let correctTargetIdentifier = "$s4main7GreeterC5greet4nameS2S_tYaKFTE"
+  _ = correctTargetIdentifier
+  let badModuleTargetIdentifier = "$s9BADMODULE7GreeterC5greet4nameS2S_tYaKFTE"
+  // the BADMODULE is a bad module, and we won't be able to find the distributed accessor
+  // this should result in a failed call, but not hang the call.
+
+  var invocation = Greeter.ActorSystem.InvocationEncoder()
+  invocation.arguments = ["BadCall"]
+  invocation.returnType = String.self
+
+  let badTarget = RemoteCallTarget(badModuleTargetIdentifier)
+
+  do {
+    // CHECK: >> remoteCall: on:main.Greeter, target:BADMODULE.Greeter.greet(name:)
+    let call = try await system.remoteCall(
+      on: local,
+      target: badTarget,
+      invocation: &invocation,
+      throwing: Never.self,
+      returning: String.self
+    )
+  } catch {
+    // CHECK: << onThrow: ExecuteDistributedTargetError(errorCode: Distributed.ExecuteDistributedTargetError.ErrorCode.targetAccessorNotFound, message: "Failed to locate distributed function accessor")
+    // CHECK: << remoteCall throw: ExecuteDistributedTargetError(errorCode: Distributed.ExecuteDistributedTargetError.ErrorCode.targetAccessorNotFound, message: "Failed to locate distributed function accessor")
+    print("caught error: \(error)")
+    print("call target was: \(badTarget.identifier)")
+    // CHECK: caught error: ExecuteDistributedTargetError(errorCode: Distributed.ExecuteDistributedTargetError.ErrorCode.targetAccessorNotFound, message: "Failed to locate distributed function accessor")
+    // CHECK: call target was: $s9BADMODULE7GreeterC5greet4nameS2S_tYaKFTE
+  }
+}
+
+@main struct Main {
+  static func main() async {
+    try! await test()
+  }
+}

test/Distributed/Runtime/distributed_actor_remoteCall.swift

@@ -493,7 +493,7 @@ func test() async throws {
     invocationDecoder: &decodeErrDecoder,
     handler: FakeResultHandler()
   )
-  // CHECK: ERROR: ExecuteDistributedTargetError(message: "Failed to decode of Int??? (for a test)")
+  // CHECK: ERROR: ExecuteDistributedTargetError(errorCode: Distributed.ExecuteDistributedTargetError.ErrorCode.other, message: "Failed to decode of Int??? (for a test)")
 
   print("done")
   // CHECK-NEXT: done