Skip to content

Commit 2439954

Browse files
committed
[Concurrency] Don't check for data races in an actor's deinit.
An actor's deinit can be invoked from any thread, and does not (cannot!) synchronize to the actor. However, because "self" is by definition unique and cannot escape, don't perform data race checking in it or any local functions/closures within the initializer. This is an imperfect approximation, because one could introduce a data race by invoking a concurrent algorithm on "self" that does not escape the closure but subverts @sendable checking and concurrently accesses actor state. However, for the moment we accept this false negative because the false positives from performing this checking are much more prevalent.
1 parent e77a27e commit 2439954

File tree

2 files changed

+60
-14
lines changed

2 files changed

+60
-14
lines changed

lib/SILGen/SILGenProlog.cpp

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,35 @@ void SILGenFunction::emitProlog(CaptureInfo captureInfo,
461461
}
462462
}
463463
}
464-
464+
465+
// Whether the given declaration context is nested within an actor's
466+
// destructor.
467+
auto isInActorDestructor = [](DeclContext *dc) {
468+
while (!dc->isModuleScopeContext() && !dc->isTypeContext()) {
469+
if (auto destructor = dyn_cast<DestructorDecl>(dc)) {
470+
switch (getActorIsolation(destructor)) {
471+
case ActorIsolation::ActorInstance:
472+
return true;
473+
474+
case ActorIsolation::GlobalActor:
475+
case ActorIsolation::GlobalActorUnsafe:
476+
// Global-actor-isolated types should likely have deinits that
477+
// are not themselves actor-isolated, yet still have access to
478+
// the instance properties of the class.
479+
return false;
480+
481+
case ActorIsolation::Independent:
482+
case ActorIsolation::Unspecified:
483+
return false;
484+
}
485+
}
486+
487+
dc = dc->getParent();
488+
}
489+
490+
return false;
491+
};
492+
465493
// Initialize ExpectedExecutor if the function is an actor-isolated
466494
// function or closure.
467495
if (auto *funcDecl =
@@ -479,7 +507,8 @@ void SILGenFunction::emitProlog(CaptureInfo captureInfo,
479507
// or are local functions. The former require a hop, while the latter
480508
// are prone to dynamic data races in code that does not enforce Sendable
481509
// completely.
482-
if (F.isAsync() || funcDecl->isLocalCapture()) {
510+
if (F.isAsync() ||
511+
(funcDecl->isLocalCapture() && !isInActorDestructor(funcDecl))) {
483512
auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
484513
ManagedValue selfArg = ManagedValue::forUnmanaged(F.getSelfArgument());
485514
ExpectedExecutor = emitLoadActorExecutor(loc, selfArg);
@@ -488,31 +517,39 @@ void SILGenFunction::emitProlog(CaptureInfo captureInfo,
488517
}
489518

490519
case ActorIsolation::GlobalActor:
491-
ExpectedExecutor =
492-
emitLoadGlobalActorExecutor(actorIsolation.getGlobalActor());
520+
if (F.isAsync() || !isInActorDestructor(funcDecl)) {
521+
ExpectedExecutor =
522+
emitLoadGlobalActorExecutor(actorIsolation.getGlobalActor());
523+
}
493524
break;
494525
}
495526
} else if (auto *closureExpr = dyn_cast<AbstractClosureExpr>(FunctionDC)) {
527+
bool wantExecutor = F.isAsync() || !isInActorDestructor(closureExpr);
496528
auto actorIsolation = closureExpr->getActorIsolation();
497529
switch (actorIsolation.getKind()) {
498530
case ClosureActorIsolation::Independent:
499531
break;
500532

501533
case ClosureActorIsolation::ActorInstance: {
502-
auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
503-
auto actorDecl = actorIsolation.getActorInstance();
504-
Type actorType = actorDecl->getType();
505-
RValue actorInstanceRV = emitRValueForDecl(loc,
506-
actorDecl, actorType, AccessSemantics::Ordinary);
507-
ManagedValue actorInstance = std::move(actorInstanceRV).getScalarValue();
508-
ExpectedExecutor = emitLoadActorExecutor(loc, actorInstance);
534+
if (wantExecutor) {
535+
auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
536+
auto actorDecl = actorIsolation.getActorInstance();
537+
Type actorType = actorDecl->getType();
538+
RValue actorInstanceRV = emitRValueForDecl(loc,
539+
actorDecl, actorType, AccessSemantics::Ordinary);
540+
ManagedValue actorInstance =
541+
std::move(actorInstanceRV).getScalarValue();
542+
ExpectedExecutor = emitLoadActorExecutor(loc, actorInstance);
543+
}
509544
break;
510545
}
511546

512547
case ClosureActorIsolation::GlobalActor:
513-
ExpectedExecutor =
514-
emitLoadGlobalActorExecutor(actorIsolation.getGlobalActor());
515-
break;
548+
if (wantExecutor) {
549+
ExpectedExecutor =
550+
emitLoadGlobalActorExecutor(actorIsolation.getGlobalActor());
551+
break;
552+
}
516553
}
517554
}
518555

test/SILGen/check_executor.swift

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ import _Concurrency
1313
// CHECK-CANONICAL: function_ref @$ss22_checkExpectedExecutor7Builtin15_filenameLength01_E7IsASCII5_line9_executoryBp_BwBi1_BwBetF
1414
@MainActor public func onMainActor() { }
1515

16+
func takeClosure(_ fn: @escaping () -> Int) { }
17+
1618
public actor MyActor {
1719
var counter = 0
1820

@@ -27,4 +29,11 @@ public actor MyActor {
2729
return self.counter
2830
}
2931
}
32+
33+
// CHECK-RAW: sil private [ossa] @$s4test7MyActorCfdSiycfU_
34+
// CHECK-RAW-NOT: extract_executor
35+
// CHECK-RAW: return [[VALUE:%.*]] : $Int
36+
deinit {
37+
takeClosure { self.counter }
38+
}
3039
}

0 commit comments

Comments
 (0)