dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.21k stars 1.57k forks source link

Expose Stream#_mayAddEvent in a synchronous interface. #44285

Open pinyin opened 3 years ago

pinyin commented 3 years ago

In my understanding, Stream._mayAddEvent is not exposed in a synchronous interface, which makes it quite inconvenient to prevent _addEventError when calling Stream#add.

Ideally, calling add on a Stream should not require the caller function to be an async function:

void addToStream(StreamController sc) {
  if(sc.mayAddEvent) sc.add(null);
}
lrhn commented 3 years ago

There is no API for checking whether an event may currently be added, synchronous or asynchronous. In almost all cases, the answer is "yes" because the event will either be delayed or buffered if it cannot currently be delieverd. There are two exceptions:

That's a very precise exception, and it requires you to be using a synchronous controller. Synchronous controllers are sharp tools, they're easy to use incorrectly, and if you do, you get unexpected errors. I recommend just making the controller non-synchronous, that's always a correct and safe behavior for a stream. Streams do not promise synchronous delivery, and synchronous controllers exist only for behind-the-scenes optimizations, not as a source of synchronous events. They're simply not built for that.

pinyin commented 3 years ago

Thanks for your reply.

There is no API for checking whether an event may currently be added, synchronous or asynchronous.

I understand that a StreamController does not guarantee whether an event is delivered to a subscription. But even though it doesn't give such guarantee, some of its subclasses (e.g. BroadcastStreamController) does throw an exception in add when _mayAddEvent is internally false. Checking isClosed does not cover the case when an addStream call is not finished.

So even when I don't care whether an event would be added, calling add may still throw in a BroadcastStreamController, which may be making the behavior quite inconsistence across asynchronous StreamController implementations.

Changing the implementation is not an option, since in 3rd-part libraries like rxdart, a BroadcastStreamController is simply used everywhere, which makes every fire-and-forget add intention quite tedious.

lrhn commented 3 years ago

Ah, yes, I forgot the "while adding stream" exception. I plan to remove that exception, but hasn't managed to complete the change yet.

Short answer: Adding a member to StreamController is a breaking change, so we won't do it unless the benefit outweighs the cost. Keeping track of how you're using the controller is the job of the owner, the controller simply reports violations by throwing.

pinyin commented 3 years ago

OK, removing the exception seems to be a good solution.

Thanks for your great work. Dart is my favorite language now, hoping it would be even better.