Skip to content

Commit 9419af7

Browse files
fix(NODE-5225): concurrent MongoClient.close() calls each attempt to close the client (#4376)
Co-authored-by: Bailey Pearson <[email protected]>
1 parent 8b2b7fd commit 9419af7

File tree

3 files changed

+98
-0
lines changed

3 files changed

+98
-0
lines changed

src/mongo_client.ts

+17
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,8 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
373373
override readonly mongoLogger: MongoLogger | undefined;
374374
/** @internal */
375375
private connectionLock?: Promise<this>;
376+
/** @internal */
377+
private closeLock?: Promise<void>;
376378

377379
/**
378380
* The consolidate, parsed, transformed and merged options.
@@ -650,6 +652,21 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
650652
* @param force - Force close, emitting no events
651653
*/
652654
async close(force = false): Promise<void> {
655+
if (this.closeLock) {
656+
return await this.closeLock;
657+
}
658+
659+
try {
660+
this.closeLock = this._close(force);
661+
await this.closeLock;
662+
} finally {
663+
// release
664+
this.closeLock = undefined;
665+
}
666+
}
667+
668+
/* @internal */
669+
private async _close(force = false): Promise<void> {
653670
// There's no way to set hasBeenClosed back to false
654671
Object.defineProperty(this.s, 'hasBeenClosed', {
655672
value: true,

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

+63
Original file line numberDiff line numberDiff line change
@@ -760,6 +760,69 @@ describe('class MongoClient', function () {
760760
});
761761
});
762762

763+
context('concurrent calls', () => {
764+
let topologyClosedSpy;
765+
beforeEach(async function () {
766+
await client.connect();
767+
const coll = client.db('db').collection('concurrentCalls');
768+
const session = client.startSession();
769+
await coll.findOne({}, { session: session });
770+
topologyClosedSpy = sinon.spy(Topology.prototype, 'close');
771+
});
772+
773+
afterEach(async function () {
774+
sinon.restore();
775+
});
776+
777+
context('when two concurrent calls to close() occur', () => {
778+
it('does not throw', async function () {
779+
await Promise.all([client.close(), client.close()]);
780+
});
781+
782+
it('clean-up logic is performed only once', async function () {
783+
await Promise.all([client.close(), client.close()]);
784+
expect(topologyClosedSpy).to.have.been.calledOnce;
785+
});
786+
});
787+
788+
context('when more than two concurrent calls to close() occur', () => {
789+
it('does not throw', async function () {
790+
await Promise.all([client.close(), client.close(), client.close(), client.close()]);
791+
});
792+
793+
it('clean-up logic is performed only once', async function () {
794+
await client.connect();
795+
await Promise.all([
796+
client.close(),
797+
client.close(),
798+
client.close(),
799+
client.close(),
800+
client.close()
801+
]);
802+
expect(topologyClosedSpy).to.have.been.calledOnce;
803+
});
804+
});
805+
806+
it('when connect rejects lock is released regardless', async function () {
807+
expect(client.topology?.isConnected()).to.be.true;
808+
809+
const closeStub = sinon.stub(client, 'close');
810+
closeStub.onFirstCall().rejects(new Error('cannot close'));
811+
812+
// first call rejected to simulate a close failure
813+
const error = await client.close().catch(error => error);
814+
expect(error).to.match(/cannot close/);
815+
816+
expect(client.topology?.isConnected()).to.be.true;
817+
closeStub.restore();
818+
819+
// second call should close
820+
await client.close();
821+
822+
expect(client.topology).to.be.undefined;
823+
});
824+
});
825+
763826
describe('active cursors', function () {
764827
let collection: Collection<{ _id: number }>;
765828
const kills = [];

test/unit/mongo_client.test.ts

+18
Original file line numberDiff line numberDiff line change
@@ -1223,4 +1223,22 @@ describe('MongoClient', function () {
12231223
});
12241224
});
12251225
});
1226+
1227+
describe('closeLock', function () {
1228+
let client;
1229+
1230+
beforeEach(async function () {
1231+
client = new MongoClient('mongodb://blah');
1232+
});
1233+
1234+
it('when client.close is pending, client.closeLock is set', () => {
1235+
client.close();
1236+
expect(client.closeLock).to.be.instanceOf(Promise);
1237+
});
1238+
1239+
it('when client.close is not pending, client.closeLock is not set', async () => {
1240+
await client.close();
1241+
expect(client.closeLock).to.be.undefined;
1242+
});
1243+
});
12261244
});

0 commit comments

Comments
 (0)