Fix: Leak of grpc resources on terminate. (#7847)

* Fix: Leak of grpc resources on terminate.

Author: MarkDuckworth

.changeset/cold-carpets-sing.md

@@ -0,0 +1,5 @@
+---
+"@firebase/firestore": patch
+---
+
+Fixed leak of grpc-js resources on terminate.

packages/firestore/src/core/component_provider.ts

@@ -483,7 +483,11 @@ export class OnlineComponentProvider {
     );
   }
 
-  terminate(): Promise<void> {
-    return remoteStoreShutdown(this.remoteStore);
+  async terminate(): Promise<void> {
+    await remoteStoreShutdown(this.remoteStore);
+
+    if (this.datastore) {
+      await this.datastore.terminate();
+    }
   }
 }

packages/firestore/src/platform/node/grpc_connection.ts

@@ -325,4 +325,17 @@ export class GrpcConnection implements Connection {
 
     return stream;
   }
+
+  /**
+   * Closes and cleans up any resources associated with the GrpcConnection.
+   * If a gRPC client has been generated for this connection, the gRPC client
+   * is closed. Failure to call terminate on a GrpcConnection can result
+   * in leaked resources of the gRPC client.
+   */
+  terminate(): void {
+    if (this.cachedStub) {
+      this.cachedStub.close();
+      this.cachedStub = undefined;
+    }
+  }
 }

packages/firestore/src/remote/connection.ts

@@ -92,6 +92,13 @@ export interface Connection {
    */
   readonly shouldResourcePathBeIncludedInRequest: boolean;
 
+  /**
+   * Closes and cleans up any resources associated with the connection. Actual
+   * resources cleaned are implementation specific. Failure to call `terminate`
+   * on a connection may result in resource leaks.
+   */
+  terminate(): void;
+
   // TODO(mcg): subscribe to connection state changes.
 }
 

packages/firestore/src/remote/datastore.ts

@@ -161,6 +161,7 @@ class DatastoreImpl extends Datastore {
 
   terminate(): void {
     this.terminated = true;
+    this.connection.terminate();
   }
 }
 

packages/firestore/src/remote/rest_connection.ts

@@ -189,4 +189,13 @@ export abstract class RestConnection implements Connection {
     );
     return `${this.baseUrl}/${RPC_URL_VERSION}/${path}:${urlRpcName}`;
   }
+
+  /**
+   * Closes and cleans up any resources associated with the connection. This
+   * implementation is a no-op because there are no resources associated
+   * with the RestConnection that need to be cleaned up.
+   */
+  terminate(): void {
+    // No-op
+  }
 }

packages/firestore/test/unit/remote/datastore.test.ts

@@ -41,6 +41,12 @@ use(chaiAsPromised);
 // `invokeRPC()` and `invokeStreamingRPC()`.
 describe('Datastore', () => {
   class MockConnection implements Connection {
+    terminateInvoked = false;
+
+    terminate(): void {
+      this.terminateInvoked = true;
+    }
+
     invokeRPC<Req, Resp>(
       rpcName: string,
       path: string,
@@ -131,6 +137,18 @@ describe('Datastore', () => {
       });
   });
 
+  it('Connection.terminate() invoked if Datastore terminated', async () => {
+    const connection = new MockConnection();
+    const datastore = newDatastore(
+      new EmptyAuthCredentialsProvider(),
+      new EmptyAppCheckTokenProvider(),
+      connection,
+      serializer
+    );
+    datastore.terminate();
+    await expect(connection.terminateInvoked).to.equal(true);
+  });
+
   it('DatastoreImpl.invokeRPC() rethrows a FirestoreError', async () => {
     const connection = new MockConnection();
     connection.invokeRPC = () =>

packages/firestore/test/unit/specs/spec_test_components.ts

@@ -254,6 +254,10 @@ export class MockConnection implements Connection {
 
   constructor(private queue: AsyncQueue) {}
 
+  terminate(): void {
+    // no-op
+  }
+
   shouldResourcePathBeIncludedInRequest: boolean = false;
 
   /**