From b88e74b532ac73160d1ba162b29ba3f6afdde413 Mon Sep 17 00:00:00 2001 From: Holly Borla Date: Fri, 9 Feb 2024 21:55:46 -0800 Subject: [PATCH] [Concurrency] Don't allow non-Sendable, isolated closures to capture non-Sendable values that may be isolated to a different actor. --- include/swift/AST/DiagnosticsSema.def | 4 + lib/Sema/TypeCheckConcurrency.cpp | 36 ++++++++- test/Concurrency/isolated_captures.swift | 98 ++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 test/Concurrency/isolated_captures.swift diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index a7ef78d8226cc..41b6878739269 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -5436,6 +5436,10 @@ ERROR(non_sendable_capture,none, "capture of %1 with non-sendable type %0 in a `@Sendable` " "%select{local function|closure}2", (Type, DeclName, bool)) +ERROR(non_sendable_isolated_capture,none, + "capture of %1 with non-sendable type %0 in an isolated " + "%select{local function|closure}2", + (Type, DeclName, bool)) ERROR(implicit_async_let_non_sendable_capture,none, "capture of %1 with non-sendable type %0 in 'async let' binding", (Type, DeclName)) diff --git a/lib/Sema/TypeCheckConcurrency.cpp b/lib/Sema/TypeCheckConcurrency.cpp index c844af5c20eaa..1451bc8526cc3 100644 --- a/lib/Sema/TypeCheckConcurrency.cpp +++ b/lib/Sema/TypeCheckConcurrency.cpp @@ -2460,6 +2460,7 @@ namespace { // which the declaration occurred, it's okay. auto decl = capture.getDecl(); auto *context = localFunc.getAsDeclContext(); + auto fnType = localFunc.getType()->getAs(); if (!mayExecuteConcurrentlyWith(context, decl->getDeclContext())) continue; @@ -2492,11 +2493,19 @@ namespace { diag::implicit_non_sendable_capture, decl->getName()); } + } else if (fnType->isSendable()) { + diagnoseNonSendableTypes(type, getDeclContext(), + /*inDerivedConformance*/Type(), + capture.getLoc(), + diag::non_sendable_capture, + decl->getName(), + /*closure=*/closure != nullptr); } else { diagnoseNonSendableTypes(type, getDeclContext(), /*inDerivedConformance*/Type(), capture.getLoc(), - diag::non_sendable_capture, decl->getName(), + diag::non_sendable_isolated_capture, + decl->getName(), /*closure=*/closure != nullptr); } } @@ -4026,15 +4035,30 @@ bool ActorIsolationChecker::mayExecuteConcurrentlyWith( if (useContext == defContext) return false; - // If both contexts are isolated to the same actor, then they will not - // execute concurrently. + bool isolatedStateMayEscape = false; + auto useIsolation = getActorIsolationOfContext( const_cast(useContext), getClosureActorIsolation); if (useIsolation.isActorIsolated()) { auto defIsolation = getActorIsolationOfContext( const_cast(defContext), getClosureActorIsolation); + // If both contexts are isolated to the same actor, then they will not + // execute concurrently. if (useIsolation == defIsolation) return false; + + // If the local function is not Sendable, its isolation differs + // from that of the context, and both contexts are actor isolated, + // then capturing non-Sendable values allows the closure to stash + // those values into actor isolated state. The original context + // may also stash those values into isolated state, enabling concurrent + // access later on. + auto &ctx = useContext->getASTContext(); + bool regionIsolationEnabled = + ctx.LangOpts.hasFeature(Feature::RegionBasedIsolation); + isolatedStateMayEscape = + (!regionIsolationEnabled && + useIsolation.isActorIsolated() && defIsolation.isActorIsolated()); } // Walk the context chain from the use to the definition. @@ -4043,6 +4067,9 @@ bool ActorIsolationChecker::mayExecuteConcurrentlyWith( if (auto closure = dyn_cast(useContext)) { if (isSendableClosure(closure, /*forActorIsolation=*/false)) return true; + + if (isolatedStateMayEscape) + return true; } if (auto func = dyn_cast(useContext)) { @@ -4050,6 +4077,9 @@ bool ActorIsolationChecker::mayExecuteConcurrentlyWith( // If the function is @Sendable... it can be run concurrently. if (func->isSendable()) return true; + + if (isolatedStateMayEscape) + return true; } } diff --git a/test/Concurrency/isolated_captures.swift b/test/Concurrency/isolated_captures.swift new file mode 100644 index 0000000000000..d3d11764c9b2a --- /dev/null +++ b/test/Concurrency/isolated_captures.swift @@ -0,0 +1,98 @@ +// RUN: %target-swift-frontend -verify -disable-availability-checking -strict-concurrency=complete -verify-additional-prefix complete- -emit-sil -o /dev/null %s +// RUN: %target-swift-frontend -verify -disable-availability-checking -strict-concurrency=complete -verify-additional-prefix region-isolation- -emit-sil -o /dev/null %s -enable-experimental-feature RegionBasedIsolation + +// REQUIRES: concurrency +// REQUIRES: asserts + +@globalActor +actor MyActor { + static let shared = MyActor() + @MyActor static var ns: NotSendable? + @MyActor static func ohNo() { ns!.x += 1 } +} + +@globalActor +actor YourActor { + static let shared = YourActor() + @YourActor static var ns: NotSendable? + @YourActor static func ohNo() { ns!.x += 1 } +} + +// expected-complete-note@+1 3{{class 'NotSendable' does not conform to the 'Sendable' protocol}} +class NotSendable { + var x: Int = 0 + + @MyActor init() { + MyActor.ns = self + } + + init(x: Int) { + self.x = x + } + + @MyActor func stash() { + MyActor.ns = self + } +} + +@MyActor func exhibitRace1() async { + let ns = NotSendable(x: 0) + MyActor.ns = ns + + // expected-region-isolation-warning@+1 {{call site passes `self` or a non-sendable argument of this function to another thread, potentially yielding a race with the caller; this is an error in Swift 6}} + await { @YourActor in + // expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in Swift 6}} + YourActor.ns = ns + }() + + await withTaskGroup(of: Void.self) { + $0.addTask { + await MyActor.ohNo() + } + + $0.addTask { + await YourActor.ohNo() + } + } +} + +@MyActor func exhibitRace2() async { + let ns = NotSendable(x: 0) + ns.stash() + + // FIXME: Region isolation should diagnose this (https://github.com/apple/swift/issues/71533) + await { @YourActor in + // expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in Swift 6}} + YourActor.ns = ns + }() + + await withTaskGroup(of: Void.self) { + $0.addTask { + await MyActor.ohNo() + } + + $0.addTask { + await YourActor.ohNo() + } + } +} + +@MyActor func exhibitRace3() async { + let ns = NotSendable() + + // FIXME: Region isolation should diagnose this (https://github.com/apple/swift/issues/71533) + await { @YourActor in + // expected-complete-warning@+1 {{capture of 'ns' with non-sendable type 'NotSendable' in an isolated closure; this is an error in Swift 6}} + YourActor.ns = ns + }() + + await withTaskGroup(of: Void.self) { + $0.addTask { + await MyActor.ohNo() + } + + $0.addTask { + await YourActor.ohNo() + } + } +}