Skip to content

Commit

Permalink
fix(AsyncSubject): fix reentrancy issue in complete
Browse files Browse the repository at this point in the history
  • Loading branch information
benlesh committed Sep 21, 2020
1 parent 30d429c commit 9e00f11
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 3 deletions.
38 changes: 38 additions & 0 deletions spec/subjects/AsyncSubject-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -191,4 +191,42 @@ describe('AsyncSubject', () => {
subject.subscribe(observer);
expect(observer.results).to.deep.equal([expected]);
});

it('should not be reentrant via complete', () => {
const subject = new AsyncSubject<number>();
let calls = 0;
subject.subscribe({
next: value => {
calls++;
if (calls < 2) {
// if this is more than 1, we're reentrant, and that's bad.
subject.complete();
}
}
});

subject.next(1);
subject.complete();

expect(calls).to.equal(1);
});

it('should not be reentrant via next', () => {
const subject = new AsyncSubject<number>();
let calls = 0;
subject.subscribe({
next: value => {
calls++;
if (calls < 2) {
// if this is more than 1, we're reentrant, and that's bad.
subject.next(value + 1);
}
}
});

subject.next(1);
subject.complete();

expect(calls).to.equal(1);
});
});
11 changes: 8 additions & 3 deletions src/internal/AsyncSubject.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/** @prettier */
import { Subject } from './Subject';
import { Subscriber } from './Subscriber';
import { Subscription } from './Subscription';
Expand All @@ -11,6 +12,7 @@ import { Subscription } from './Subscription';
export class AsyncSubject<T> extends Subject<T> {
private value: T | null = null;
private hasValue = false;
private isComplete = false;

protected checkFinalizedStatuses(subscriber: Subscriber<T>) {
const { hasError, hasValue, value, thrownError, isStopped } = this;
Expand All @@ -30,8 +32,11 @@ export class AsyncSubject<T> extends Subject<T> {
}

complete(): void {
const { hasValue, value } = this;
hasValue && super.next(value!);
super.complete();
const { hasValue, value, isComplete } = this;
if (!isComplete) {
this.isComplete = true;
hasValue && super.next(value!);
super.complete();
}
}
}

0 comments on commit 9e00f11

Please sign in to comment.