[sending] Mark Task.init, Task.detached and friends as taking a sending closure instead of a __owned @Sendable #74239
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gottesmm
requested review from
ktoso,
hborla,
xedin and
slavapestov
as code owners
gottesmm
force-pushed
the
taskinit-sending
branch
3 times, most recently
from
ca2599d to
89fd614
Compare
|
@swift-ci smoke test |
gottesmm
force-pushed
the
taskinit-sending
branch
from
80c27a4 to
c2f3e67
Compare
|
@swift-ci smoke test |
…uming parameter The reason why I am doing this is that: 1. We don't support no escaping closure parameters today and would like to leave the design space open. 2. These closures are bitwise copyable/trivial so marking them as specifically consuming doesn't really make sense.
…now inferred to be nonisolated.
Consider the following piece of code and what the isolation is of the closure
literal passed to doSomething():
```swift
func doSomething(_ f: sending () -> ()) { ... }
@MyCustomActor
func foo() async {
doSomething {
// What is the isolation here?
}
}
```
In this case, the isolation of the closure is @MyCustomActor. This is because
non-Sendable closures are by default isolated to their current context (in this
case @MyCustomActor since foo is @MyCustomActor isolated). This is a problem
since
1. Our closure is a synchronous function that does not have the ability to hop
to MyCustomActor to run said code. This could result in a concurrency hole
caused by running the closure in doSomething() without hopping to
MyCustomActor's executor.
2. In Region Based Isolation, a closure that is actor isolated cannot be sent,
so we would immediately hit a region isolation error.
To fix this issue, by default, if a closure literal is passed as a sending
parameter, we make its isolation nonisolated. This ensures that it is
disconnected and can be transferred safely.
In the case of an async closure literal, we follow the same semantics, but we
add an additional wrinkle: we keep support of inheritActorIsolation. If one
marks an async closure literal with inheritActorIsolation, we allow for it to be
passed as a sendable parameter since it is actually Sendable under the hood.
…endable function. We view the conversion from a Sendable to a non-Sendable function via convert function to produce a new fresh sendable value. We should squelch that error.
…properly mark actor isolated async closures passed to an inheritActorContext argument as being Sendable. The reason why I am fixing this is that otherwise, we get a warning when one creates an actor isolated closure and pass it into a task, e.x.: ```swift @mainactor func test() { // We would get a warning on the closure below saying that we are sending // a closure that is MainActor isolated. Task { ... } } ```
…tead of __owned @sendable so we can capture non-isolated values.
… obsolete in swift 6. These APIs remained to preserve source compatibility during bringup. We do not want them used in swift 6 mode... so deprecate in swift 5 and obsolete in swift 6.
…oSendingParameter. This came up while I was talking with @xedin. This name makes it really clear what we are trying to communicate to the author.
There is only one call site of this helper function, so we are not gaining any flexibility by doing this. It only makes it harder to reason about what the function actually does without providing any benefit. I am doing this before I make in the next commit an actual semantic changing commit to ease review.
…g DeclContexts. Without this, we were hitting weird behaviors in flow_isolation.swift only when compiling in Swift 5 mode. Consider the following example: ``` func fakeTask(@_inheritActorContext _ x: sending @escaping () async -> ()) {} actor MyActor { var x: Int = 5 var y: Int = 6 var hax: MyActor? = nil init() { fakeTask { // Warning! You can remove await // Error! Cannot access actor state in nonisolated closure. _ = await self.hax } } } ``` The reason why this was happening is that we were not understanding that the closure passed to fakeTask is not part of the init and has different isolation from the init (it is nonisolated but does not have access to the init's state). This then caused us to exit early and not properly process the isolation crossing point there. Now, we properly understand that a sending inheritActorContext closure (just like an @sendable closure) has this property in actor initializers.
…ding non-Sendable function instead of an @sendable function. This matches the interface of the public stdlib APIs that wrap these builtin calls.
gottesmm
force-pushed
the
taskinit-sending
branch
from
c2f3e67 to
d28eef2
Compare
gottesmm
force-pushed
the
taskinit-sending
branch
from
420e7a3 to
3685cdd
Compare
|
@swift-ci smoke test |
|
Hello @gottesmm, I'm pretty happy that this pull request has shipped in Xcode 16 beta 4. 🙏 I was expecting that |
Michael's PR here is part of the implementation for SE-0430, which only includes adoption of That said:
It's a known issue. |
harlanhaskins
pushed a commit
to harlanhaskins/swift
that referenced
this pull request
Jul 29, 2024
This was deprecated in swiftlang#74239 with a message to replace with ```swift Task { @mainactor in ... } ``` But that doesn't have equivalent semantics. `startOnMainActor` executes the closure synchronously up to the first suspension point, which is necessary for working with APIs like XPC connections and UIKit drag-and-drop.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Just creating the PR now for discussion. Going to add more here.
Resolves: #74382