diff --git a/.changeset/cold-carpets-sing.md b/.changeset/cold-carpets-sing.md new file mode 100644 index 00000000000..e291792fb5b --- /dev/null +++ b/.changeset/cold-carpets-sing.md @@ -0,0 +1,5 @@ +--- +"@firebase/firestore": patch +--- + +Fixed leak of grpc-js resources on terminate. diff --git a/packages/firestore/src/core/component_provider.ts b/packages/firestore/src/core/component_provider.ts index 447e0f07857..54d16cc1655 100644 --- a/packages/firestore/src/core/component_provider.ts +++ b/packages/firestore/src/core/component_provider.ts @@ -483,7 +483,11 @@ export class OnlineComponentProvider { ); } - terminate(): Promise { - return remoteStoreShutdown(this.remoteStore); + async terminate(): Promise { + await remoteStoreShutdown(this.remoteStore); + + if (this.datastore) { + await this.datastore.terminate(); + } } } diff --git a/packages/firestore/src/platform/node/grpc_connection.ts b/packages/firestore/src/platform/node/grpc_connection.ts index 77879c280ad..387a612b381 100644 --- a/packages/firestore/src/platform/node/grpc_connection.ts +++ b/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; + } + } } diff --git a/packages/firestore/src/remote/connection.ts b/packages/firestore/src/remote/connection.ts index 717f2e9dbd2..34f458f1a1d 100644 --- a/packages/firestore/src/remote/connection.ts +++ b/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. } diff --git a/packages/firestore/src/remote/datastore.ts b/packages/firestore/src/remote/datastore.ts index 9c85a48bf63..c39311f1aa1 100644 --- a/packages/firestore/src/remote/datastore.ts +++ b/packages/firestore/src/remote/datastore.ts @@ -161,6 +161,7 @@ class DatastoreImpl extends Datastore { terminate(): void { this.terminated = true; + this.connection.terminate(); } } diff --git a/packages/firestore/src/remote/rest_connection.ts b/packages/firestore/src/remote/rest_connection.ts index 4d15976ea54..d76dd88d2fb 100644 --- a/packages/firestore/src/remote/rest_connection.ts +++ b/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 + } } diff --git a/packages/firestore/test/unit/remote/datastore.test.ts b/packages/firestore/test/unit/remote/datastore.test.ts index 795bf59edaf..a64f0f8405f 100644 --- a/packages/firestore/test/unit/remote/datastore.test.ts +++ b/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( 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 = () => diff --git a/packages/firestore/test/unit/specs/spec_test_components.ts b/packages/firestore/test/unit/specs/spec_test_components.ts index 83512392147..2a2e480de63 100644 --- a/packages/firestore/test/unit/specs/spec_test_components.ts +++ b/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; /**