From 4662d49477fdf0b5ef01f8d4f8aed8b87d77ea66 Mon Sep 17 00:00:00 2001 From: Pavel Jbanov Date: Mon, 10 Mar 2014 10:01:48 -0400 Subject: [PATCH] fix(scope): should allow removing listener during an event Closes #695 --- lib/core/scope.dart | 33 ++++++++++++++++++++++++++++----- test/core/scope_spec.dart | 10 ++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/lib/core/scope.dart b/lib/core/scope.dart index 98d2d4378..eb14dc711 100644 --- a/lib/core/scope.dart +++ b/lib/core/scope.dart @@ -740,6 +740,9 @@ class ScopeStream extends async.Stream { final _Streams _streams; final String _name; final subscriptions = []; + bool _firing = false; + List _toRemove; + ScopeStream(this._streams, this._exceptionHandler, this._name); @@ -754,16 +757,36 @@ class ScopeStream extends async.Stream { } void _fire(ScopeEvent event) { - for (ScopeStreamSubscription subscription in subscriptions) { - try { - subscription._onData(event); - } catch (e, s) { - _exceptionHandler(e, s); + _firing = true; + try { + for (ScopeStreamSubscription subscription in subscriptions) { + try { + subscription._onData(event); + } catch (e, s) { + _exceptionHandler(e, s); + } + } + } finally { + _firing = false; + if (_toRemove != null) { + _toRemove.forEach(_actuallyRemove); + _toRemove = null; } } } void _remove(ScopeStreamSubscription subscription) { + if (_firing) { + if (_toRemove == null) { + _toRemove = []; + } + _toRemove.add(subscription); + } else { + _actuallyRemove(subscription); + } + } + + void _actuallyRemove(ScopeStreamSubscription subscription) { assert(subscription._scopeStream == this); if (subscriptions.remove(subscription)) { if (subscriptions.isEmpty) _streams._addCount(_name, -1); diff --git a/test/core/scope_spec.dart b/test/core/scope_spec.dart index 5932621dc..70bcffb2b 100644 --- a/test/core/scope_spec.dart +++ b/test/core/scope_spec.dart @@ -681,6 +681,16 @@ void main() { expect(args.length).toBe(4); expect(args).toEqual(['do', 're', 'me', 'fa']); })); + + it('should allow removing listener during an event', inject((RootScope rootScope) { + StreamSubscription subscription; + subscription = rootScope.on('foo').listen((_) { + subscription.cancel(); + }); + expect(() { + rootScope.broadcast('foo'); + }).not.toThrow(); + })); }); }); });