fix(NODE-5225): concurrent MongoClient.close() calls each attempt to …

…close the client (#4376)

Co-authored-by: Bailey Pearson <[email protected]>

src/mongo_client.ts

@@ -373,6 +373,8 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
   override readonly mongoLogger: MongoLogger | undefined;
   /** @internal */
   private connectionLock?: Promise<this>;
+  /** @internal */
+  private closeLock?: Promise<void>;
 
   /**
    * The consolidate, parsed, transformed and merged options.
@@ -650,6 +652,21 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
    * @param force - Force close, emitting no events
    */
   async close(force = false): Promise<void> {
+    if (this.closeLock) {
+      return await this.closeLock;
+    }
+
+    try {
+      this.closeLock = this._close(force);
+      await this.closeLock;
+    } finally {
+      // release
+      this.closeLock = undefined;
+    }
+  }
+
+  /* @internal */
+  private async _close(force = false): Promise<void> {
     // There's no way to set hasBeenClosed back to false
     Object.defineProperty(this.s, 'hasBeenClosed', {
       value: true,

test/integration/node-specific/mongo_client.test.ts

@@ -760,6 +760,69 @@ describe('class MongoClient', function () {
       });
     });
 
+    context('concurrent calls', () => {
+      let topologyClosedSpy;
+      beforeEach(async function () {
+        await client.connect();
+        const coll = client.db('db').collection('concurrentCalls');
+        const session = client.startSession();
+        await coll.findOne({}, { session: session });
+        topologyClosedSpy = sinon.spy(Topology.prototype, 'close');
+      });
+
+      afterEach(async function () {
+        sinon.restore();
+      });
+
+      context('when two concurrent calls to close() occur', () => {
+        it('does not throw', async function () {
+          await Promise.all([client.close(), client.close()]);
+        });
+
+        it('clean-up logic is performed only once', async function () {
+          await Promise.all([client.close(), client.close()]);
+          expect(topologyClosedSpy).to.have.been.calledOnce;
+        });
+      });
+
+      context('when more than two concurrent calls to close() occur', () => {
+        it('does not throw', async function () {
+          await Promise.all([client.close(), client.close(), client.close(), client.close()]);
+        });
+
+        it('clean-up logic is performed only once', async function () {
+          await client.connect();
+          await Promise.all([
+            client.close(),
+            client.close(),
+            client.close(),
+            client.close(),
+            client.close()
+          ]);
+          expect(topologyClosedSpy).to.have.been.calledOnce;
+        });
+      });
+
+      it('when connect rejects lock is released regardless', async function () {
+        expect(client.topology?.isConnected()).to.be.true;
+
+        const closeStub = sinon.stub(client, 'close');
+        closeStub.onFirstCall().rejects(new Error('cannot close'));
+
+        // first call rejected to simulate a close failure
+        const error = await client.close().catch(error => error);
+        expect(error).to.match(/cannot close/);
+
+        expect(client.topology?.isConnected()).to.be.true;
+        closeStub.restore();
+
+        // second call should close
+        await client.close();
+
+        expect(client.topology).to.be.undefined;
+      });
+    });
+
     describe('active cursors', function () {
       let collection: Collection<{ _id: number }>;
       const kills = [];

test/unit/mongo_client.test.ts

@@ -1223,4 +1223,22 @@ describe('MongoClient', function () {
       });
     });
   });
+
+  describe('closeLock', function () {
+    let client;
+
+    beforeEach(async function () {
+      client = new MongoClient('mongodb://blah');
+    });
+
+    it('when client.close is pending, client.closeLock is set', () => {
+      client.close();
+      expect(client.closeLock).to.be.instanceOf(Promise);
+    });
+
+    it('when client.close is not pending, client.closeLock is not set', async () => {
+      await client.close();
+      expect(client.closeLock).to.be.undefined;
+    });
+  });
 });