Skip to content

Commit 0d6c9cd

Browse files
vkarpov15nbbeeken
andauthored
fix(NODE-5691): make findOne() close implicit session to avoid memory leak (#3889)
Co-authored-by: Neal Beeken <[email protected]>
1 parent df0780e commit 0d6c9cd

File tree

2 files changed

+101
-3
lines changed

2 files changed

+101
-3
lines changed

src/collection.ts

+4-1
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,10 @@ export class Collection<TSchema extends Document = Document> {
493493
filter: Filter<TSchema> = {},
494494
options: FindOptions = {}
495495
): Promise<WithId<TSchema> | null> {
496-
return this.find(filter, options).limit(-1).batchSize(1).next();
496+
const cursor = this.find(filter, options).limit(-1).batchSize(1);
497+
const res = await cursor.next();
498+
await cursor.close();
499+
return res;
497500
}
498501

499502
/**

test/integration/crud/crud_api.test.ts

+97-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,19 @@
11
import { expect } from 'chai';
22
import { on } from 'events';
3-
4-
import { type MongoClient, MongoError, ObjectId, ReturnDocument } from '../../mongodb';
3+
import * as semver from 'semver';
4+
import * as sinon from 'sinon';
5+
6+
import {
7+
Collection,
8+
CommandFailedEvent,
9+
CommandSucceededEvent,
10+
type MongoClient,
11+
MongoError,
12+
MongoServerError,
13+
ObjectId,
14+
ReturnDocument
15+
} from '../../mongodb';
16+
import { type FailPoint } from '../../tools/utils';
517
import { assert as test } from '../shared';
618

719
// instanceof cannot be use reliably to detect the new models in js due to scoping and new
@@ -31,6 +43,8 @@ describe('CRUD API', function () {
3143
});
3244

3345
afterEach(async function () {
46+
sinon.restore();
47+
3448
await client?.close();
3549
client = null;
3650

@@ -61,6 +75,87 @@ describe('CRUD API', function () {
6175
await client.close();
6276
});
6377

78+
describe('findOne()', () => {
79+
let client: MongoClient;
80+
let events;
81+
let collection: Collection<{ _id: number }>;
82+
83+
beforeEach(async function () {
84+
client = this.configuration.newClient({ monitorCommands: true });
85+
events = [];
86+
client.on('commandSucceeded', commandSucceeded =>
87+
commandSucceeded.commandName === 'find' ? events.push(commandSucceeded) : null
88+
);
89+
client.on('commandFailed', commandFailed =>
90+
commandFailed.commandName === 'find' ? events.push(commandFailed) : null
91+
);
92+
93+
collection = client.db('findOne').collection('findOne');
94+
await collection.drop().catch(() => null);
95+
await collection.insertMany([{ _id: 1 }, { _id: 2 }]);
96+
});
97+
98+
afterEach(async () => {
99+
await collection.drop().catch(() => null);
100+
await client.close();
101+
});
102+
103+
describe('when the operation succeeds', () => {
104+
it('the cursor for findOne is closed', async function () {
105+
const spy = sinon.spy(Collection.prototype, 'find');
106+
const result = await collection.findOne({});
107+
expect(result).to.deep.equal({ _id: 1 });
108+
expect(events.at(0)).to.be.instanceOf(CommandSucceededEvent);
109+
expect(spy.returnValues.at(0)).to.have.property('closed', true);
110+
expect(spy.returnValues.at(0)).to.have.nested.property('session.hasEnded', true);
111+
});
112+
});
113+
114+
describe('when the find operation fails', () => {
115+
beforeEach(async function () {
116+
if (semver.lt(this.configuration.version, '4.2.0')) {
117+
if (this.currentTest) {
118+
this.currentTest.skipReason = `Cannot run fail points on server version: ${this.configuration.version}`;
119+
}
120+
return this.skip();
121+
}
122+
123+
const failPoint: FailPoint = {
124+
configureFailPoint: 'failCommand',
125+
mode: 'alwaysOn',
126+
data: {
127+
failCommands: ['find'],
128+
// 1 == InternalError, but this value not important to the test
129+
errorCode: 1
130+
}
131+
};
132+
await client.db().admin().command(failPoint);
133+
});
134+
135+
afterEach(async function () {
136+
if (semver.lt(this.configuration.version, '4.2.0')) {
137+
return;
138+
}
139+
140+
const failPoint: FailPoint = {
141+
configureFailPoint: 'failCommand',
142+
mode: 'off',
143+
data: { failCommands: ['find'] }
144+
};
145+
await client.db().admin().command(failPoint);
146+
});
147+
148+
it('the cursor for findOne is closed', async function () {
149+
const spy = sinon.spy(Collection.prototype, 'find');
150+
const error = await collection.findOne({}).catch(error => error);
151+
expect(error).to.be.instanceOf(MongoServerError);
152+
expect(events.at(0)).to.be.instanceOf(CommandFailedEvent);
153+
expect(spy.returnValues.at(0)).to.have.property('closed', true);
154+
expect(spy.returnValues.at(0)).to.have.nested.property('session.hasEnded', true);
155+
});
156+
});
157+
});
158+
64159
context('when creating a cursor with find', () => {
65160
let collection;
66161

0 commit comments

Comments
 (0)