Skip to content

Enable Sendability for AsyncStream and AsyncThrowingStream #41713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 8, 2022

Conversation

phausler
Copy link
Contributor

@phausler phausler commented Mar 8, 2022

This makes AsyncStream and AsyncThrowingStream both conditionally Sendable when their Element is Sendable and removes the runtime assertion associated with that behavior.

@phausler phausler requested a review from ktoso March 8, 2022 00:46
@phausler
Copy link
Contributor Author

phausler commented Mar 8, 2022

@swift-ci please smoke test

@phausler
Copy link
Contributor Author

phausler commented Mar 8, 2022

@swift-ci Please Test Source Compatibility

@grynspan
Copy link
Contributor

grynspan commented Mar 8, 2022

@swift-ci please test linux

@@ -341,7 +340,7 @@ extension AsyncThrowingStream {
}
}

func cancel() {
@Sendable func cancel() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that one necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so since it is used in the termination which is sendable

Copy link
Contributor

@ktoso ktoso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM

@phausler
Copy link
Contributor Author

phausler commented Mar 8, 2022

@swift-ci please smoke test

@phausler
Copy link
Contributor Author

phausler commented Mar 8, 2022

@swift-ci please test linux

@phausler phausler merged commit d5f63ee into swiftlang:main Mar 8, 2022
} else {
unlock()
fatalError("attempt to await next() on more than one task")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link

@malhal malhal Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phausler I'd like the fatalError back please.

jamieQ added a commit to jamieQ/swift that referenced this pull request Aug 14, 2024
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
jamieQ added a commit to jamieQ/swift that referenced this pull request Aug 14, 2024
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
jamieQ added a commit to jamieQ/swift that referenced this pull request Aug 15, 2024
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
jamieQ added a commit to jamieQ/swift that referenced this pull request Aug 17, 2024
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
jamieQ added a commit to jamieQ/swift that referenced this pull request Aug 20, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants