-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Stream.asBroadcastStream() is broken semantically #11289
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
Comments
Added Area-Library, Triaged labels. |
Stream.asBroadcastStream() is basically implemented as We will add named Removed Type-Defect label. |
More details: The subscription will be a wrapped version of the actual subscription. It will not allow to change onData, onError and onDone. |
Will the default onCancel cancel the underlying subscription? I think this is a bad idea. asBroadcastStream() should not have surprise behavior where it "dies" if it ever temporarily loses all of its listeners. If someone wants that behavior, they should specify it in their onCancel explicitly. What about: asBroadcastStream({persistWithoutListeners: true}); or some well-named parameter? |
The default onCancel will no longer cancel the original subscription. With the onCancel callback available, we can let the user decide when to cancel, and we don't need a hacky heuristic any more. That does mean that some existing uses of asBroadcastStream will now not close the source stream, but wait for it to run dry. It's likely that it wasn't a deliberate choice before, so we don't think it will be a problem. Many uses of asBroadcastStream were on a StreamController's strem, and they are better rewritten as using a broadcast StreamController directly. Added Started label. |
Set owner to @lrhn. |
Added Fixed label. |
The semantics of Stream.asBroadcastStream() result in something that is deeply counterintuitive.
A broadcast stream is supposed to support any number of listeners. This is indeed true of Stream.asBroadcastStream() - it can be listened to any number of times.
However, if for some reason its listener count ever falls to 0, it will automatically unsubscribe from its backing single-subscriber stream, rendering it forevermore useless.
This results in bugs in real-world code and hacks like calling .drain() on the result of .asBroadcastStream() to add a permanent listener to hold the stream open. This behavior is also substantially different from new StreamController.broadcast() which behaves properly in this case.
The text was updated successfully, but these errors were encountered: