Enable Sendability for AsyncStream and AsyncThrowingStream #41713
Conversation
|
@swift-ci please smoke test |
|
@swift-ci Please Test Source Compatibility |
|
@swift-ci please test linux |
| @@ -341,7 +340,7 @@ extension AsyncThrowingStream { | |||
| } | |||
| } | |||
|
|
|||
| func cancel() { | |||
| @Sendable func cancel() { | |||
There was a problem hiding this comment.
I think so since it is used in the termination which is sendable
|
@swift-ci please smoke test |
|
@swift-ci please test linux |
| } else { | ||
| unlock() | ||
| fatalError("attempt to await next() on more than one task") |
There was a problem hiding this comment.
@phausler apologies for resurrecting this, but if you have a chance, could you help me understand why this check was removed? i assume it was not directly related to the conditional Sendable conformance since the same change wasn't made to the throwing variant, but maybe i'm overlooking something.
There was a problem hiding this comment.
@phausler I'd like the fatalError back please.
There was a problem hiding this comment.
I have a contrary opinion on this.
I would like this same behavior/changeset to be applied to the AsyncThrowingStream._Storage.
I believe that consuming a stream from multiple tasks should not be an error. The current behavior of AsyncStream in that scenario is to distribute the stream elements to each consumer somewhat randomly - which I think is perfectly reasonable behavior.
I don't think fatalError in this circumstance is necessary.
I'm sympathetic to the argument that multi-consumption is a violation of this API and should fail fast to alert the user to a possible bug. But I also believe that multi-consumption ought to be a feature - so if it isn't this API then it deserves to be first-classed in some other way.
Removing a fatalError here seems like a relatively low-risk (?) way to make progress on multi-consumption.
At any rate, AsyncStream and AsyncThrowing stream should definitely have the same behavior in a multi-consumer scenario - and right now they do not.
There was a problem hiding this comment.
There was a problem hiding this comment.
Multi-consumption means shared mutable state and the tool for that is actor.
There was a problem hiding this comment.
I'm not sure I understand what that has to do with the fatalError here?
Do you mean that instead of a lock and a check for multiple access, this should just be implemented with an actor? I would agree with that.
There was a problem hiding this comment.
I was just responding to your statement that said multi consumer should be a feature, I just think it's impossible similar to how value types can't be copied and continue to have the same changes.
I mean if you want multi consumption you need to go reference type, and an actor would help with storing the mutable state that is all the clients.
As of the changes in swiftlang#41713 to enable Sendability for AsyncStream, it has been possible to create multiple stream consumers operating concurrently. This change fixes behavior in the case that the underlying stream is terminated while multiple pending continuations are outstanding. Previously such consumers would have been leaked (never resumed). Now, they are notified of the stream's termination and resumed appropriately. Resolves swiftlang#66541 & swiftlang#71412
As of the changes in swiftlang#41713 to enable Sendability for AsyncStream, it has been possible to create multiple stream consumers operating concurrently. This change fixes behavior in the case that the underlying stream is terminated while multiple pending continuations are outstanding. Previously such consumers would have been leaked (never resumed). Now, they are notified of the stream's termination and resumed appropriately. Resolves swiftlang#66541 & swiftlang#71412
As of the changes in swiftlang#41713 to enable Sendability for AsyncStream, it has been possible to create multiple stream consumers operating concurrently. This change fixes behavior in the case that the underlying stream is terminated while multiple pending continuations are outstanding. Previously such consumers would have been leaked (never resumed). Now, they are notified of the stream's termination and resumed appropriately. Resolves swiftlang#66541 & swiftlang#71412
As of the changes in swiftlang#41713 to enable Sendability for AsyncStream, it has been possible to create multiple stream consumers operating concurrently. This change fixes behavior in the case that the underlying stream is terminated while multiple pending continuations are outstanding. Previously such consumers would have been leaked (never resumed). Now, they are notified of the stream's termination and resumed appropriately. Resolves swiftlang#66541 & swiftlang#71412
As of the changes in swiftlang#41713 to enable Sendability for AsyncStream, it has been possible to create multiple stream consumers operating concurrently. This change fixes behavior in the case that the underlying stream is terminated while multiple pending continuations are outstanding. Previously such consumers would have been leaked (never resumed). Now, they are notified of the stream's termination and resumed appropriately. Resolves swiftlang#66541 & swiftlang#71412
This makes AsyncStream and AsyncThrowingStream both conditionally Sendable when their Element is Sendable and removes the runtime assertion associated with that behavior.