Skip to content

Commit 0ac9c1f

Browse files
committed
Fix: Leak of grpc resources on terminate.
1 parent efa116b commit 0ac9c1f

File tree

6 files changed

+38
-2
lines changed

6 files changed

+38
-2
lines changed

packages/firestore/src/core/component_provider.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,11 @@ export class OnlineComponentProvider {
483483
);
484484
}
485485

486-
terminate(): Promise<void> {
487-
return remoteStoreShutdown(this.remoteStore);
486+
async terminate(): Promise<void> {
487+
await remoteStoreShutdown(this.remoteStore);
488+
489+
if (this.datastore) {
490+
await this.datastore.terminate();
491+
}
488492
}
489493
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,4 +325,11 @@ export class GrpcConnection implements Connection {
325325

326326
return stream;
327327
}
328+
329+
terminate(): void {
330+
if (this.cachedStub) {
331+
this.cachedStub.close();
332+
this.cachedStub = undefined;
333+
}
334+
}
328335
}

packages/firestore/src/remote/connection.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,8 @@ export interface Connection {
9292
*/
9393
readonly shouldResourcePathBeIncludedInRequest: boolean;
9494

95+
terminate(): void;
96+
9597
// TODO(mcg): subscribe to connection state changes.
9698
}
9799

packages/firestore/src/remote/datastore.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ class DatastoreImpl extends Datastore {
161161

162162
terminate(): void {
163163
this.terminated = true;
164+
this.connection.terminate();
164165
}
165166
}
166167

packages/firestore/src/remote/rest_connection.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,4 +189,8 @@ export abstract class RestConnection implements Connection {
189189
);
190190
return `${this.baseUrl}/${RPC_URL_VERSION}/${path}:${urlRpcName}`;
191191
}
192+
193+
terminate(): void {
194+
// No-op
195+
}
192196
}

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ use(chaiAsPromised);
4141
// `invokeRPC()` and `invokeStreamingRPC()`.
4242
describe('Datastore', () => {
4343
class MockConnection implements Connection {
44+
terminateInvoked = false;
45+
46+
terminate(): void {
47+
this.terminateInvoked = true;
48+
}
49+
4450
invokeRPC<Req, Resp>(
4551
rpcName: string,
4652
path: string,
@@ -131,6 +137,18 @@ describe('Datastore', () => {
131137
});
132138
});
133139

140+
it('Connection.terminate() invoked if Datastore terminated', async () => {
141+
const connection = new MockConnection();
142+
const datastore = newDatastore(
143+
new EmptyAuthCredentialsProvider(),
144+
new EmptyAppCheckTokenProvider(),
145+
connection,
146+
serializer
147+
);
148+
datastore.terminate();
149+
await expect(connection.terminateInvoked).to.equal(true);
150+
});
151+
134152
it('DatastoreImpl.invokeRPC() rethrows a FirestoreError', async () => {
135153
const connection = new MockConnection();
136154
connection.invokeRPC = () =>

0 commit comments

Comments
 (0)