Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/cold-carpets-sing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@firebase/firestore": patch
---

Fixed leak of grpc-js resources on terminate.
8 changes: 6 additions & 2 deletions packages/firestore/src/core/component_provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
13 changes: 13 additions & 0 deletions packages/firestore/src/platform/node/grpc_connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
7 changes: 7 additions & 0 deletions packages/firestore/src/remote/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
}

Expand Down
1 change: 1 addition & 0 deletions packages/firestore/src/remote/datastore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ class DatastoreImpl extends Datastore {

terminate(): void {
this.terminated = true;
this.connection.terminate();
}
}

Expand Down
9 changes: 9 additions & 0 deletions packages/firestore/src/remote/rest_connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
18 changes: 18 additions & 0 deletions packages/firestore/test/unit/remote/datastore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 = () =>
Expand Down
4 changes: 4 additions & 0 deletions packages/firestore/test/unit/specs/spec_test_components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ export class MockConnection implements Connection {

constructor(private queue: AsyncQueue) {}

terminate(): void {
// no-op
}

shouldResourcePathBeIncludedInRequest: boolean = false;

/**
Expand Down