Sdk: StreamController Extension

[Peng-Qian]

Hi,
I found the following methods are quite useful to control stream.
Currently, I create an extension on StreamController, is it possible to add these methods in StreamController?

  void mayAdd(T event) {
    if (!isClosed && hasListener) {
      add(event);
    }
  }

  void mayAddError(Object error, [StackTrace? stackTrace]){
    if (!isClosed && hasListener) {
      addError(error, stackTrace);
    }
  }

  Future<void> mayClose() async {
    if (!isClosed && !hasListener) {
      await close();
    }
  }

[mraleph]

In general adding new member to public interfaces in Dart core libraries is considered a breaking change and thus most likely non-starter (at least until after default interface methods - which are nowhere near the top list of features).

But I defer to @lrhn.

[lrhn]

It's true that we are unlikely to add methods to interfaces. The StreamController interface is one that you'd expect to be safe, because who'd want to re-implement that, but we do see classes implementing the interface, which would make adding new members a breaking change.

Apart from that, I'm not sure those particular methods are ones that we would make generally available.
They are probably somewhat specific to your way of using streams, and parts of them seem unnecessary.
I'd remove all the hasListener checks. They either do nothing (because the underlying controller will do nothing if it doesn't have listeners anyway), or they prevent adding elements before a single-subscription stream is listened to, which might be surprising, or the mayClose not doing anything if there is no current listener is definitely wrong for broadcast streams.
It's also unnecessary to check isClosed in mayClose since calling close a second time already does nothing.

So, this really boils down to:

void tryAdd(T event) {
  if (!isClosed) add(event);
}
void tryAddError(Object error, [StackTrace? stack]) {
  if (!isClosed) addError(error, stack);
}

I don't think they are important enough to do breaking change for.

[Peng-Qian]

Hi @mraleph @lrhn
Thx for your reply and explanation.
My use case is using a broadcast stream to monitor data changes(crud) in the infrastructure layer, and I try to close streams as efficiently as possible. So I do onCancel like following:

    Future<void> onCancel() async {
      if (controller != null && !controller.hasListener) {
        await controller.close();
      }
    }

with mayClose:

    Future<void> onCancel() async {
        await controller?.mayClose();
    }

and the controller may add events when the stream is closed, so I do

    controller?.mayAdd(value);

Maybe it is just my particular use, anyway, thanks for your time~

[lrhn]

If the onCancel is on the stream of controller, then I can absolutely gurantee that controller.hasListener will be false when onCancel is called, both for single-subscription and broadcast streams. Not having a listener is what triggers onCancel being called.
What you do is equivalent to await controller?.close(). It's safe to call close twice, so there is no need to check isClosed first.

If it is not a broadcast stream, it's unnecessary to close the controller. The one and only listener has already stopped listening, so noone will get the done event.

For the remaining uses, I think they derive from the way you are using controllers, but they are not general enough that we'd want to have them on the controller for everybody.

[Peng-Qian]

Hey @irhn,
I still wanna confirm that for the non-broadcast stream, we don't need to close the controller to release the memory? I thought we should always consider closing the StreamController, like bloc.close() in Bloc.

[lrhn]

If your non-broadcast stream controller receives an onCancel, then nobody is listening to the stream any more.
Any further calls to add, addError, addStream or close will throw away the event. There is no advantage in calling close as a final good-bye to the controller, you can just let it be garbage collected as-is.

You shouldn't worry about "clean-up" apart from what onCancel should be doing anyway. There is no internal state in the controller at this point that you can do anything about. Calling close will just set the isClosed bit. (Normally it would set the isClosed bit and add a "done" event to the StreamSubscription, but the subscription has been cancelled).