From 60e7521196bc6fb14d5d0280e1c7163dfc2333ac Mon Sep 17 00:00:00 2001 From: Michal Skvely Date: Sun, 29 Apr 2018 13:29:02 +0200 Subject: [PATCH 1/2] test(delay): failing test --- spec/operators/delay-spec.ts | 38 +++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/spec/operators/delay-spec.ts b/spec/operators/delay-spec.ts index 83d2b7e769..67777b9a9f 100644 --- a/spec/operators/delay-spec.ts +++ b/spec/operators/delay-spec.ts @@ -1,9 +1,12 @@ -import * as Rx from 'rxjs/Rx'; +import { Observable, timer } from 'rxjs'; +import { delay, repeatWhen, skip, take, tap, finalize } from 'rxjs/operators'; +import { TestScheduler } from 'rxjs/testing'; +import * as sinon from 'sinon'; +import { expect } from 'chai'; import { hot, cold, expectObservable, expectSubscriptions, time } from '../helpers/marble-testing'; declare const asDiagram: Function; -declare const rxTestScheduler: Rx.TestScheduler; -const Observable = Rx.Observable; +declare const rxTestScheduler: TestScheduler; /** @test {delay} */ describe('Observable.prototype.delay', () => { @@ -143,4 +146,33 @@ describe('Observable.prototype.delay', () => { expectObservable(result).toBe(expected); }); + + it('should unsubscribe scheduled action when result is unsubscribed explicitly', () => { + let subscribeSpy: any = null; + const counts: number[] = []; + + const e1 = cold('a|'); + const expected = '--a-(a|)'; + const duration = time('-|'); + const result = e1.pipe( + repeatWhen(notifications => { + const delayed = notifications.pipe(delay(duration, rxTestScheduler)); + subscribeSpy = sinon.spy(delayed['source'], 'subscribe'); + return delayed; + }), + skip(1), + take(2), + tap({ + next() { + const [[subscriber]] = subscribeSpy.args; + counts.push(subscriber._subscriptions.length); + }, + complete() { + expect(counts).to.deep.equal([1, 1]); + } + }) + ); + + expectObservable(result).toBe(expected); + }); }); From 8afb49277d64e85b95d9125db014a4b062c21de0 Mon Sep 17 00:00:00 2001 From: Michal Skvely Date: Sun, 29 Apr 2018 13:29:13 +0200 Subject: [PATCH 2/2] fix(delay): fix memory leak --- spec/operators/delay-spec.ts | 8 ++++---- src/internal/operators/delay.ts | 1 + 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/spec/operators/delay-spec.ts b/spec/operators/delay-spec.ts index 67777b9a9f..2a042c31af 100644 --- a/spec/operators/delay-spec.ts +++ b/spec/operators/delay-spec.ts @@ -1,5 +1,5 @@ -import { Observable, timer } from 'rxjs'; -import { delay, repeatWhen, skip, take, tap, finalize } from 'rxjs/operators'; +import { Observable } from 'rxjs'; +import { delay, repeatWhen, skip, take, tap } from 'rxjs/operators'; import { TestScheduler } from 'rxjs/testing'; import * as sinon from 'sinon'; import { expect } from 'chai'; @@ -147,12 +147,12 @@ describe('Observable.prototype.delay', () => { expectObservable(result).toBe(expected); }); - it('should unsubscribe scheduled action when result is unsubscribed explicitly', () => { + it('should unsubscribe scheduled actions after execution', () => { let subscribeSpy: any = null; const counts: number[] = []; const e1 = cold('a|'); - const expected = '--a-(a|)'; + const expected = '--a-(a|)'; const duration = time('-|'); const result = e1.pipe( repeatWhen(notifications => { diff --git a/src/internal/operators/delay.ts b/src/internal/operators/delay.ts index 0f786e759a..bd0586f23e 100644 --- a/src/internal/operators/delay.ts +++ b/src/internal/operators/delay.ts @@ -92,6 +92,7 @@ class DelaySubscriber extends Subscriber { const delay = Math.max(0, queue[0].time - scheduler.now()); this.schedule(state, delay); } else { + this.unsubscribe(); source.active = false; } }