Skip to content

Commit 011d2d2

Browse files
committed
[Concurrency/Distributed] nonisolated-nonsending by default breaks distributed thunks
the new NonisolatedNonsendingByDefault upcoming feature breaks remote calls in distributed actors, because the expected isolation doesn't match and the runtime swift_distributed_execute_target_resume will crash. This is a short term fix to unblock adopters, however preferably we should mark the thunks as nonisolated(nonsending), though that seems to be more involved. resolves rdar://159247975
1 parent 5f20a5b commit 011d2d2

File tree

6 files changed

+95
-2
lines changed

6 files changed

+95
-2
lines changed

include/swift/AST/Decl.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3001,6 +3001,13 @@ class ValueDecl : public Decl {
30013001
/// `distributed var get { }` accessors.
30023002
bool isDistributedGetAccessor() const;
30033003

3004+
/// Is this a 'distributed thunk'?
3005+
///
3006+
/// Distributed thunks are synthesized functions which perform the "is remote?"
3007+
/// check, before dispatching to a 'system.remoteCall' (if actor was remote).
3008+
/// They are always 'async' and 'throws'.
3009+
bool isDistributedThunk() const;
3010+
30043011
bool hasName() const { return bool(Name); }
30053012
bool isOperator() const { return Name.isOperator(); }
30063013

lib/AST/DistributedDecl.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,6 +1369,13 @@ bool ValueDecl::isDistributedGetAccessor() const {
13691369
return false;
13701370
}
13711371

1372+
bool ValueDecl::isDistributedThunk() const {
1373+
if (auto func = dyn_cast<AbstractFunctionDecl>(this)) {
1374+
return func->isDistributedThunk();
1375+
}
1376+
return false;
1377+
}
1378+
13721379
ConstructorDecl *
13731380
NominalTypeDecl::getDistributedRemoteCallTargetInitFunction() const {
13741381
auto mutableThis = const_cast<NominalTypeDecl *>(this);

lib/Sema/CodeSynthesisDistributedActor.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -737,6 +737,7 @@ static FuncDecl *createSameSignatureDistributedThunkDecl(DeclContext *DC,
737737

738738
thunk->setSynthesized(true);
739739
thunk->setDistributedThunk(true);
740+
// TODO(distributed): These would benefit from becoming nonisolated(nonsending)
740741
thunk->getAttrs().add(NonisolatedAttr::createImplicit(C));
741742

742743
return thunk;

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5231,6 +5231,13 @@ getIsolationFromAttributes(const Decl *decl, bool shouldDiagnose = true,
52315231
if (decl->getASTContext().LangOpts.hasFeature(
52325232
Feature::NonisolatedNonsendingByDefault)) {
52335233
if (auto *value = dyn_cast<ValueDecl>(decl)) {
5234+
// TODO(distributed): make distributed thunks nonisolated(nonsending) and remove this if
5235+
if (value->isAsync() && value->isDistributedThunk()) {
5236+
// don't change isolation of distributed thunks until we make them nonisolated(nonsending),
5237+
// since the runtime calling them assumes they're just nonisolated right now.
5238+
return ActorIsolation::forNonisolated(nonisolatedAttr->isUnsafe());
5239+
}
5240+
52345241
if (value->isAsync() &&
52355242
value->getModuleContext() == decl->getASTContext().MainModule) {
52365243
return ActorIsolation::forCallerIsolationInheriting();

test/Distributed/Runtime/distributed_actor_remoteCall_roundtrip.swift

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
// RUN: %target-codesign %t/a.out
55
// RUN: %target-run %t/a.out | %FileCheck %s --enable-var-scope
66

7-
// X: %target-run-simple-swift( -Xfrontend -module-name=main -target %target-swift-5.7-abi-triple -parse-as-library -Xfrontend -I -Xfrontend %t ) | %FileCheck %s
8-
97
// REQUIRES: executable_test
108
// REQUIRES: concurrency
119
// REQUIRES: distributed
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend-emit-module -emit-module-path %t/FakeDistributedActorSystems.swiftmodule \
3+
// RUN: -module-name FakeDistributedActorSystems -target %target-swift-5.7-abi-triple \
4+
// RUN: %S/../Inputs/FakeDistributedActorSystems.swift
5+
6+
// RUN: %target-build-swift -module-name main -enable-upcoming-feature NonisolatedNonsendingByDefault \
7+
// RUN: -target %target-swift-5.7-abi-triple -j2 -parse-as-library -I %t %s \
8+
// RUN: %S/../Inputs/FakeDistributedActorSystems.swift -o %t/a.out
9+
10+
// RUN: %target-codesign %t/a.out
11+
// RUN: %target-run %t/a.out | %FileCheck %s --enable-var-scope
12+
13+
// REQUIRES: swift_feature_NonisolatedNonsendingByDefault
14+
15+
// REQUIRES: executable_test
16+
// REQUIRES: concurrency
17+
// REQUIRES: distributed
18+
19+
// rdar://76038845
20+
// UNSUPPORTED: use_os_stdlib
21+
// UNSUPPORTED: back_deployment_runtime
22+
23+
// FIXME(distributed): Distributed actors currently have some issues on windows, isRemote always returns false. rdar://82593574
24+
// UNSUPPORTED: OS=windows-msvc
25+
26+
import Distributed
27+
import FakeDistributedActorSystems
28+
29+
typealias DefaultDistributedActorSystem = FakeRoundtripActorSystem
30+
31+
distributed actor Greeter: CustomStringConvertible {
32+
distributed func echo(name: String) -> String {
33+
return "Echo: \(name) (impl on: \(self.id))"
34+
}
35+
36+
distributed func error() throws -> String {
37+
throw SomeError()
38+
}
39+
40+
nonisolated var description: String {
41+
"\(Self.self)(\(id))"
42+
}
43+
}
44+
45+
struct SomeError: Error {}
46+
47+
// ==== Test -------------------------------------------------------------------
48+
49+
func test() async throws {
50+
let system = DefaultDistributedActorSystem()
51+
52+
let local = Greeter(actorSystem: system)
53+
let ref = try Greeter.resolve(id: local.id, using: system)
54+
55+
let reply = try await ref.echo(name: "Caplin")
56+
// CHECK: > encode argument name:name, value: Caplin
57+
// CHECK-NOT: > encode error type
58+
// CHECK: > encode return type: Swift.String
59+
// CHECK: > done recording
60+
// CHECK: >> remoteCall
61+
// CHECK: > decode return type: Swift.String
62+
// CHECK: > decode argument: Caplin
63+
// CHECK: << onReturn: Echo: Caplin (impl on: ActorAddress(address: "<unique-id>"))
64+
65+
print("got: \(reply)")
66+
// CHECK: got: Echo: Caplin (impl on: ActorAddress(address: "<unique-id>"))
67+
}
68+
69+
@main struct Main {
70+
static func main() async {
71+
try! await test()
72+
}
73+
}

0 commit comments

Comments
 (0)