Skip to content

Commit f732661

Browse files
committed
[Concurrency] Model actor isolation to an arbitrary actor instance using
a VarDecl or Expr. This generalization exposed a bug where distributed actor isolation checking was skipped in some cases, including for the isolated call in `whenLocal`. The `whenLocal` implementation violated distributed actor isolation because despite the `__isLocal` dynamic check, the `self` value passed to the `body` function argument is still not statically local. To workaround this, I applied the `_local` modifier explicitly to `self` before the call, which also necessitated allowing `_local` to be written explicitly in the Distributed library.
1 parent 0e8ffef commit f732661

File tree

6 files changed

+72
-12
lines changed

6 files changed

+72
-12
lines changed

include/swift/AST/ActorIsolation.h

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class ActorIsolation {
7070

7171
private:
7272
union {
73-
llvm::PointerUnion<NominalTypeDecl *, VarDecl *> actorInstance;
73+
llvm::PointerUnion<NominalTypeDecl *, VarDecl *, Expr *> actorInstance;
7474
Type globalActor;
7575
void *pointer;
7676
};
@@ -84,7 +84,9 @@ class ActorIsolation {
8484

8585
ActorIsolation(Kind kind, NominalTypeDecl *actor, unsigned parameterIndex);
8686

87-
ActorIsolation(Kind kind, VarDecl *capturedActor);
87+
ActorIsolation(Kind kind, VarDecl *actor, unsigned parameterIndex);
88+
89+
ActorIsolation(Kind kind, Expr *actor, unsigned parameterIndex);
8890

8991
ActorIsolation(Kind kind, Type globalActor)
9092
: globalActor(globalActor), kind(kind), isolatedByPreconcurrency(false),
@@ -113,8 +115,18 @@ class ActorIsolation {
113115
return ActorIsolation(ActorInstance, actor, parameterIndex + 1);
114116
}
115117

118+
static ActorIsolation forActorInstanceParameter(VarDecl *actor,
119+
unsigned parameterIndex) {
120+
return ActorIsolation(ActorInstance, actor, parameterIndex + 1);
121+
}
122+
123+
static ActorIsolation forActorInstanceParameter(Expr *actor,
124+
unsigned parameterIndex) {
125+
return ActorIsolation(ActorInstance, actor, parameterIndex + 1);
126+
}
127+
116128
static ActorIsolation forActorInstanceCapture(VarDecl *capturedActor) {
117-
return ActorIsolation(ActorInstance, capturedActor);
129+
return ActorIsolation(ActorInstance, capturedActor, 0);
118130
}
119131

120132
static ActorIsolation forGlobalActor(Type globalActor) {
@@ -179,6 +191,8 @@ class ActorIsolation {
179191

180192
VarDecl *getActorInstance() const;
181193

194+
Expr *getActorInstanceExpr() const;
195+
182196
bool isGlobalActor() const {
183197
return getKind() == GlobalActor;
184198
}

lib/AST/Decl.cpp

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11223,18 +11223,35 @@ ActorIsolation::ActorIsolation(Kind kind, NominalTypeDecl *actor,
1122311223
: actorInstance(actor), kind(kind), isolatedByPreconcurrency(false),
1122411224
silParsed(false), parameterIndex(parameterIndex) {}
1122511225

11226-
ActorIsolation::ActorIsolation(Kind kind, VarDecl *capturedActor)
11227-
: actorInstance(capturedActor), kind(kind), isolatedByPreconcurrency(false),
11228-
silParsed(false), parameterIndex(0) {}
11226+
ActorIsolation::ActorIsolation(Kind kind, VarDecl *actor,
11227+
unsigned parameterIndex)
11228+
: actorInstance(actor), kind(kind), isolatedByPreconcurrency(false),
11229+
silParsed(false), parameterIndex(parameterIndex) {}
11230+
11231+
ActorIsolation::ActorIsolation(Kind kind, Expr *actor,
11232+
unsigned parameterIndex)
11233+
: actorInstance(actor), kind(kind), isolatedByPreconcurrency(false),
11234+
silParsed(false), parameterIndex(parameterIndex) {}
1122911235

1123011236
NominalTypeDecl *ActorIsolation::getActor() const {
1123111237
assert(getKind() == ActorInstance);
1123211238

1123311239
if (silParsed)
1123411240
return nullptr;
1123511241

11242+
Type actorType;
11243+
1123611244
if (auto *instance = actorInstance.dyn_cast<VarDecl *>()) {
11237-
return instance->getTypeInContext()
11245+
actorType = instance->getTypeInContext();
11246+
} else if (auto *instance = actorInstance.dyn_cast<Expr *>()) {
11247+
actorType = instance->getType();
11248+
}
11249+
11250+
if (actorType) {
11251+
if (auto wrapped = actorType->getOptionalObjectType()) {
11252+
actorType = wrapped;
11253+
}
11254+
return actorType
1123811255
->getReferenceStorageReferent()->getAnyActor();
1123911256
}
1124011257

@@ -11250,6 +11267,15 @@ VarDecl *ActorIsolation::getActorInstance() const {
1125011267
return actorInstance.dyn_cast<VarDecl *>();
1125111268
}
1125211269

11270+
Expr *ActorIsolation::getActorInstanceExpr() const {
11271+
assert(getKind() == ActorInstance);
11272+
11273+
if (silParsed)
11274+
return nullptr;
11275+
11276+
return actorInstance.dyn_cast<Expr *>();
11277+
}
11278+
1125311279
bool ActorIsolation::isMainActor() const {
1125411280
if (silParsed)
1125511281
return false;

lib/AST/Type.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,10 @@ NominalTypeDecl *TypeBase::getAnyActor() {
549549
return nullptr;
550550
}
551551

552+
if (auto self = getAs<DynamicSelfType>()) {
553+
return self->getSelfType()->getAnyActor();
554+
}
555+
552556
// Existential types: check for Actor protocol.
553557
if (isExistentialType()) {
554558
auto layout = getExistentialLayout();

lib/Sema/TypeCheckAttr.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6833,7 +6833,14 @@ void AttributeChecker::visitDistributedActorAttr(DistributedActorAttr *attr) {
68336833
}
68346834

68356835
void AttributeChecker::visitKnownToBeLocalAttr(KnownToBeLocalAttr *attr) {
6836-
if (!D->isImplicit()) {
6836+
auto &ctx = D->getASTContext();
6837+
auto *module = D->getDeclContext()->getParentModule();
6838+
auto *distributed = ctx.getLoadedModule(ctx.Id_Distributed);
6839+
6840+
// FIXME: An explicit `_local` is used in the implementation of
6841+
// `DistributedActor.whenLocal`, which otherwise violates actor
6842+
// isolation checking.
6843+
if (!D->isImplicit() && (module != distributed)) {
68376844
diagnoseAndRemoveAttr(attr, diag::distributed_local_cannot_be_used);
68386845
}
68396846
}

lib/Sema/TypeCheckConcurrency.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3290,16 +3290,19 @@ namespace {
32903290
// that can talk about specific parameters.
32913291
auto nominal = getType(arg)->getAnyNominal();
32923292
if (!nominal) {
3293+
// FIXME: This is wrong for distributed actors.
32933294
nominal = getType(arg)->getASTContext().getProtocol(
32943295
KnownProtocolKind::Actor);
32953296
}
32963297

3298+
auto origArg = const_cast<Expr *>(arg->findOriginalValue());
3299+
32973300
// FIXME: Also allow 'Optional.none'
3298-
if (dyn_cast<NilLiteralExpr>(arg->findOriginalValue())) {
3301+
if (dyn_cast<NilLiteralExpr>(origArg)) {
32993302
mayExitToNonisolated = true;
33003303
} else {
33013304
unsatisfiedIsolation =
3302-
ActorIsolation::forActorInstanceParameter(nominal, paramIdx);
3305+
ActorIsolation::forActorInstanceParameter(origArg, paramIdx);
33033306
mayExitToNonisolated = false;
33043307
}
33053308

@@ -3447,6 +3450,11 @@ namespace {
34473450
Type optionalAnyActorType = isolationExpr->getType();
34483451
switch (isolation) {
34493452
case ActorIsolation::ActorInstance: {
3453+
if (auto *instance = isolation.getActorInstanceExpr()) {
3454+
actorExpr = instance;
3455+
break;
3456+
}
3457+
34503458
const VarDecl *var = isolation.getActorInstance();
34513459
if (!var) {
34523460
auto dc = getDeclContext();
@@ -4690,7 +4698,7 @@ ActorIsolation ActorIsolationRequest::evaluate(
46904698
actorType = paramType;
46914699
}
46924700
if (auto actor = actorType->getAnyActor())
4693-
return ActorIsolation::forActorInstanceParameter(actor, *paramIdx);
4701+
return ActorIsolation::forActorInstanceParameter(param, *paramIdx);
46944702
}
46954703

46964704
// Diagnose global state that is not either immutable plus Sendable or

stdlib/public/Distributed/DistributedActor.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,8 @@ extension DistributedActor {
366366
_ body: @Sendable (isolated Self) async throws -> T
367367
) async rethrows -> T? {
368368
if __isLocalActor(self) {
369-
return try await body(self)
369+
_local let localSelf = self
370+
return try await body(localSelf)
370371
} else {
371372
return nil
372373
}

0 commit comments

Comments
 (0)