Skip to content

Commit 54242a1

Browse files
Merge pull request #28 from Workiva/rap-1780-single-subscription-streams
RAP-1780 Handle single-subscription stream controllers better
2 parents ab65d71 + 57679b0 commit 54242a1

File tree

2 files changed

+43
-7
lines changed

2 files changed

+43
-7
lines changed

lib/src/disposable/disposable.dart

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ class _InternalDisposable implements _Disposable {
3030

3131
@override
3232
Future<Null> dispose() {
33-
var disposeFuture = _disposer();
33+
var disposeFuture = _disposer != null ? _disposer() : null;
3434
_disposer = null;
3535
if (disposeFuture == null) {
36-
return new Future(() => null);
36+
return new Future.value();
3737
}
3838
return disposeFuture.then((_) => null);
3939
}
@@ -300,12 +300,27 @@ class Disposable implements _Disposable, DisposableManagerV3 {
300300
@override
301301
void manageStreamController(StreamController controller) {
302302
_throwOnInvalidCall('manageStreamController', 'controller', controller);
303-
_internalDisposables.add(new _InternalDisposable(() {
304-
if (!controller.hasListener) {
303+
// If a single-subscription stream has a subscription and that
304+
// subscription is subsequently canceled, the `done` future will
305+
// complete, but there is no other way for us to tell that this
306+
// is what has happened. If we then listen to the stream (since
307+
// closing a stream that was never listened to never completes) we'll
308+
// get an exception. This workaround allows us to "know" when a
309+
// subscription has been canceled so we don't bother trying to
310+
// listen to the stream before closing it.
311+
bool isDone = false;
312+
var disposable = new _InternalDisposable(() {
313+
if (!controller.hasListener && !controller.isClosed && !isDone) {
305314
controller.stream.listen((_) {});
306315
}
307316
return controller.close();
308-
}));
317+
});
318+
controller.done.then((_) {
319+
isDone = true;
320+
_internalDisposables.remove(disposable);
321+
disposable.dispose();
322+
});
323+
_internalDisposables.add(disposable);
309324
}
310325

311326
@mustCallSuper

test/unit/vm/disposable_test.dart

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,29 @@ void main() {
262262
});
263263

264264
test(
265-
'should close a single-subscription stream with no listener'
266-
'when parent is disposed', () async {
265+
'should complete normally for a single-subscription stream, with '
266+
'a listener, that has been closed when parent is disposed', () async {
267+
var controller = new StreamController();
268+
var sub = controller.stream.listen(expectAsync1((_) {}, count: 0));
269+
thing.manageStreamController(controller);
270+
await controller.close();
271+
await thing.dispose();
272+
await sub.cancel();
273+
});
274+
275+
test(
276+
'should complete normally for a single-subscription stream with a '
277+
'canceled listener when parent is disposed', () async {
278+
var controller = new StreamController();
279+
var sub = controller.stream.listen(expectAsync1((_) {}, count: 0));
280+
thing.manageStreamController(controller);
281+
await sub.cancel();
282+
await thing.dispose();
283+
});
284+
285+
test(
286+
'should close a single-subscription stream that never had a '
287+
'listener when parent is disposed', () async {
267288
var controller = new StreamController();
268289
thing.manageStreamController(controller);
269290
expect(controller.isClosed, isFalse);

0 commit comments

Comments
 (0)