Skip to content

Commit

Permalink
fix(NODE-5225): eagerly clear MongoClient.topology in MongoClient.clo…
Browse files Browse the repository at this point in the history
…se()

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 (<anonymous>)
  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.<anonymous> (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
  • Loading branch information
alecgibson committed Apr 24, 2023
1 parent 325c4bc commit d185520
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
10 changes: 5 additions & 5 deletions src/mongo_client.ts
Expand Up @@ -504,10 +504,14 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> {
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) {
Expand All @@ -522,10 +526,6 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> {
}
}

// clear out references to old topology
const topology = this.topology;
this.topology = undefined;

await new Promise<void>((resolve, reject) => {
topology.close({ force }, error => {
if (error) return reject(error);
Expand Down
8 changes: 8 additions & 0 deletions test/integration/node-specific/mongo_client.test.ts
Expand Up @@ -385,6 +385,14 @@ describe('class MongoClient', function () {
}
});

it('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 () {
Expand Down

0 comments on commit d185520

Please sign in to comment.