From 0d7b7c14e34eed43fb2ad1386281800fa3ae8aec Mon Sep 17 00:00:00 2001
From: Nicholas Jamieson <nicholas@cartant.com>
Date: Mon, 18 May 2020 11:39:31 +1000
Subject: [PATCH] fix: finalize callback will be called after the source
 observable is torn down.

* test: add failing finalize call-order test

* fix: call finalize callback after unsubscription

* chore: remove unused import

* test: add finalize-after-teardown test

* fix: add finalize callback in operator call

Closes #5357
---
 spec/operators/finalize-spec.ts    | 35 +++++++++++++++++++++++++++++-
 src/internal/operators/finalize.ts | 17 +++------------
 2 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/spec/operators/finalize-spec.ts b/spec/operators/finalize-spec.ts
index dda6c449b6..fffdfda5d7 100644
--- a/spec/operators/finalize-spec.ts
+++ b/spec/operators/finalize-spec.ts
@@ -2,7 +2,7 @@ import { expect } from 'chai';
 import { finalize, map, share } from 'rxjs/operators';
 import { TestScheduler } from 'rxjs/testing';
 import { hot, cold, expectObservable, expectSubscriptions } from '../helpers/marble-testing';
-import { of, timer, interval, NEVER } from 'rxjs';
+import { of, timer, interval, NEVER, Observable } from 'rxjs';
 import { asInteropObservable } from '../helpers/interop-helper';
 
 declare const type: Function;
@@ -172,4 +172,37 @@ describe('finalize operator', () => {
     subscription.unsubscribe();
     expect(finalized).to.be.true;
   });
+
+  it('should finalize sources before sinks', () => {
+    const finalized: string[] = [];
+    of(42).pipe(
+      finalize(() => finalized.push('source')),
+      finalize(() => finalized.push('sink'))
+    ).subscribe();
+    expect(finalized).to.deep.equal(['source', 'sink']);
+  });
+
+  it('should finalize after the teardown', () => {
+    const order: string[] = [];
+    const source = new Observable<void>(() => {
+      return () => order.push('teardown');
+    });
+    const subscription = source.pipe(
+      finalize(() => order.push('finalize'))
+    ).subscribe();
+    subscription.unsubscribe();
+    expect(order).to.deep.equal(['teardown', 'finalize']);
+  });
+
+  it('should finalize after the teardown with synchronous completion', () => {
+    const order: string[] = [];
+    const source = new Observable<void>(subscriber => {
+      subscriber.complete();
+      return () => order.push('teardown');
+    });
+    source.pipe(
+      finalize(() => order.push('finalize'))
+    ).subscribe();
+    expect(order).to.deep.equal(['teardown', 'finalize']);
+  });
 });
diff --git a/src/internal/operators/finalize.ts b/src/internal/operators/finalize.ts
index 864f8e3bc1..11febe684d 100644
--- a/src/internal/operators/finalize.ts
+++ b/src/internal/operators/finalize.ts
@@ -1,6 +1,5 @@
 import { Operator } from '../Operator';
 import { Subscriber } from '../Subscriber';
-import { Subscription } from '../Subscription';
 import { Observable } from '../Observable';
 import { MonoTypeOperatorFunction, TeardownLogic } from '../types';
 import { subscribeWith } from '../util/subscribeWith';
@@ -69,18 +68,8 @@ class FinallyOperator<T> implements Operator<T, T> {
   }
 
   call(subscriber: Subscriber<T>, source: any): TeardownLogic {
-    return subscribeWith(source, new FinallySubscriber(subscriber, this.callback));
-  }
-}
-
-/**
- * We need this JSDoc comment for affecting ESDoc.
- * @ignore
- * @extends {Ignored}
- */
-class FinallySubscriber<T> extends Subscriber<T> {
-  constructor(destination: Subscriber<T>, callback: () => void) {
-    super(destination);
-    this.add(new Subscription(callback));
+    const subscription = subscribeWith(source, subscriber);
+    subscription.add(this.callback);
+    return subscription;
   }
 }