diff --git a/spec/Subscriber-spec.ts b/spec/Subscriber-spec.ts index de8a8d2bca..cf78ac27e0 100644 --- a/spec/Subscriber-spec.ts +++ b/spec/Subscriber-spec.ts @@ -97,4 +97,17 @@ describe('Subscriber', () => { expect(argument).to.have.lengthOf(0); }); + + it('should have idempotent unsubscription', () => { + let count = 0; + const subscriber = new Subscriber(); + subscriber.add(() => ++count); + expect(count).to.equal(0); + + subscriber.unsubscribe(); + expect(count).to.equal(1); + + subscriber.unsubscribe(); + expect(count).to.equal(1); + }); }); diff --git a/spec/Subscription-spec.ts b/spec/Subscription-spec.ts index 21d769f666..2544e57751 100644 --- a/spec/Subscription-spec.ts +++ b/spec/Subscription-spec.ts @@ -159,5 +159,17 @@ describe('Subscription', () => { done(); }); }); + + it('Should have idempotent unsubscription', () => { + let count = 0; + const subscription = new Subscription(() => ++count); + expect(count).to.equal(0); + + subscription.unsubscribe(); + expect(count).to.equal(1); + + subscription.unsubscribe(); + expect(count).to.equal(1); + }); }); }); diff --git a/src/internal/Subscription.ts b/src/internal/Subscription.ts index 1a07ffa508..0c97812fec 100644 --- a/src/internal/Subscription.ts +++ b/src/internal/Subscription.ts @@ -40,7 +40,8 @@ export class Subscription implements SubscriptionLike { */ constructor(unsubscribe?: () => void) { if (unsubscribe) { - ( this)._unsubscribe = unsubscribe; + (this as any)._ctorUnsubscribe = true; + (this as any)._unsubscribe = unsubscribe; } } @@ -57,7 +58,7 @@ export class Subscription implements SubscriptionLike { return; } - let { _parentOrParents, _unsubscribe, _subscriptions } = ( this); + let { _parentOrParents, _ctorUnsubscribe, _unsubscribe, _subscriptions } = (this as any); this.closed = true; this._parentOrParents = null; @@ -75,6 +76,18 @@ export class Subscription implements SubscriptionLike { } if (isFunction(_unsubscribe)) { + // It's only possible to null _unsubscribe - to release the reference to + // any teardown function passed in the constructor - if the property was + // actually assigned in the constructor, as there are some classes that + // are derived from Subscriber (which derives from Subscription) that + // implement an _unsubscribe method as a mechanism for obtaining + // unsubscription notifications and some of those subscribers are + // recycled. Also, in some of those subscribers, _unsubscribe switches + // from a prototype method to an instance property - see notifyNext in + // RetryWhenSubscriber. + if (_ctorUnsubscribe) { + (this as any)._unsubscribe = undefined; + } try { _unsubscribe.call(this); } catch (e) { @@ -131,7 +144,7 @@ export class Subscription implements SubscriptionLike { add(teardown: TeardownLogic): Subscription { let subscription = (teardown); - if (!(teardown)) { + if (!teardown) { return Subscription.EMPTY; }