From e2c1447c5bade7ce154c018739cd35707e0db81d Mon Sep 17 00:00:00 2001 From: Alec Gibson <12036746+alecgibson@users.noreply.github.com> Date: Mon, 24 Apr 2023 12:10:46 +0100 Subject: [PATCH] fix(NODE-5225): eagerly clear MongoClient.topology in MongoClient.close() At the moment, calling `MongoClient.close()` in quick succession leads to an error: ``` TypeError: Cannot read properties of undefined (reading 'close') at node-mongodb-native/src/mongo_client.ts:530:16 at new Promise () at LegacyMongoClient.close (src/mongo_client.ts:529:11) at processTicksAndRejections (node:internal/process/task_queues:95:5) at async Promise.all (index 1) at async Context. (test/integration/node-specific/mongo_client.test.ts:393:5) ``` This happens because: 1. The first call attempts to asynchronously [close][1] the session pool sessions 2. While this is happening, the second call passes the topology [null check][2] (since `this.topology` hasn't yet been [unset][3], and tries to also close session pool sessions 3. Both calls then set [`const topology = this.topology`][4] but the second call will get undefined since the first call [unset][3] `this.topology` 4. The second call now tries to call [`close()`][5] on an `undefined` `topology` and throws This change fixes the issue by moving the `topology` unset immediately after the `null` check, so we know it's always correctly set. [1]: https://github.com/mongodb/node-mongodb-native/blob/325c4bc37decdf12e957bfad8bd4ee4d28b1bf95/src/mongo_client.ts#L516 [2]: https://github.com/mongodb/node-mongodb-native/blob/325c4bc37decdf12e957bfad8bd4ee4d28b1bf95/src/mongo_client.ts#L503 [3]: https://github.com/mongodb/node-mongodb-native/blob/325c4bc37decdf12e957bfad8bd4ee4d28b1bf95/src/mongo_client.ts#L527 [4]: https://github.com/mongodb/node-mongodb-native/blob/325c4bc37decdf12e957bfad8bd4ee4d28b1bf95/src/mongo_client.ts#L526 [5]: https://github.com/mongodb/node-mongodb-native/blob/325c4bc37decdf12e957bfad8bd4ee4d28b1bf95/src/mongo_client.ts#L530 --- src/mongo_client.ts | 10 +++++----- test/integration/node-specific/mongo_client.test.ts | 8 ++++++++ 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/mongo_client.ts b/src/mongo_client.ts index d7779aeb47..8e56c4f7ae 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -504,10 +504,14 @@ export class MongoClient extends TypedEventEmitter { return; } + // clear out references to old topology + const topology = this.topology; + this.topology = undefined; + // If we would attempt to select a server and get nothing back we short circuit // to avoid the server selection timeout. const selector = readPreferenceServerSelector(ReadPreference.primaryPreferred); - const topologyDescription = this.topology.description; + const topologyDescription = topology.description; const serverDescriptions = Array.from(topologyDescription.servers.values()); const servers = selector(topologyDescription, serverDescriptions); if (servers.length !== 0) { @@ -522,10 +526,6 @@ export class MongoClient extends TypedEventEmitter { } } - // clear out references to old topology - const topology = this.topology; - this.topology = undefined; - await new Promise((resolve, reject) => { topology.close({ force }, error => { if (error) return reject(error); diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index f92ccee05a..2955e41293 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -385,6 +385,14 @@ describe('class MongoClient', function () { } }); + it('can call close() concurrently', async function () { + const client = this.configuration.newClient(); + await client.connect(); + // Ensure topology is opened before trying to close + await client.db().command({ hello: 1 }); + await Promise.all([client.close(), client.close()]); + }); + context('explict #connect()', () => { let client: MongoClient; beforeEach(function () {