Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cursor): close underlying query cursor when calling destroy() #14982

Merged
merged 3 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions lib/cursor/aggregationCursor.js
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,37 @@ AggregationCursor.prototype.close = async function close() {
this.emit('close');
};

/**
* Marks this cursor as destroyed. Will stop streaming and subsequent calls to
* `next()` will error.
*
* @return {this}
* @api private
* @method _destroy
*/

AggregationCursor.prototype._destroy = function _destroy(_err, callback) {
let waitForCursor = null;
if (!this.cursor) {
waitForCursor = new Promise((resolve) => {
this.once('cursor', resolve);
});
} else {
waitForCursor = Promise.resolve();
}

waitForCursor
.then(() => this.cursor.close())
.then(() => {
this._closed = false;
callback();
})
.catch(error => {
callback(error);
});
return this;
};

/**
* Get the next document from this cursor. Will return `null` when there are
* no documents left.
Expand Down
33 changes: 33 additions & 0 deletions lib/cursor/queryCursor.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,39 @@ QueryCursor.prototype.close = async function close() {
}
};

/**
* Marks this cursor as destroyed. Will stop streaming and subsequent calls to
* `next()` will error.
*
* @return {this}
* @api private
* @method _destroy
*/

QueryCursor.prototype._destroy = function _destroy(_err, callback) {
let waitForCursor = null;
if (!this.cursor) {
waitForCursor = new Promise((resolve) => {
this.once('cursor', resolve);
});
} else {
waitForCursor = Promise.resolve();
}

waitForCursor
.then(() => {
this.cursor.close();
})
.then(() => {
this._closed = false;
callback();
})
.catch(error => {
callback(error);
});
return this;
};

/**
* Rewind this cursor to its uninitialized state. Any options that are present on the cursor will
* remain in effect. Iterating this cursor will cause new queries to be sent to the server, even
Expand Down
29 changes: 29 additions & 0 deletions test/query.cursor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

'use strict';

const { once } = require('events');
const start = require('./common');

const assert = require('assert');
Expand Down Expand Up @@ -920,6 +921,34 @@ describe('QueryCursor', function() {
assert.ok(cursor.cursor);
assert.equal(driverCursor, cursor.cursor);
});

it('handles destroy() (gh-14966)', async function() {
db.deleteModel(/Test/);
const TestModel = db.model('Test', mongoose.Schema({ name: String }));

const stream = await TestModel.find().cursor();
await once(stream, 'cursor');
assert.ok(!stream.cursor.closed);

stream.destroy();

await once(stream.cursor, 'close');
assert.ok(stream.destroyed);
assert.ok(stream.cursor.closed);
});

it('handles destroy() before cursor is created (gh-14966)', async function() {
db.deleteModel(/Test/);
const TestModel = db.model('Test', mongoose.Schema({ name: String }));

const stream = await TestModel.find().cursor();
assert.ok(!stream.cursor);
stream.destroy();

await once(stream, 'cursor');
assert.ok(stream.destroyed);
assert.ok(stream.cursor.closed);
});
});

async function delay(ms) {
Expand Down
6 changes: 6 additions & 0 deletions types/cursor.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ declare module 'mongoose' {
*/
close(): Promise<void>;

/**
* Destroy this cursor, closing the underlying cursor. Will stop streaming
* and subsequent calls to `next()` will error.
*/
destroy(): this;

/**
* Rewind this cursor to its uninitialized state. Any options that are present on the cursor will
* remain in effect. Iterating this cursor will cause new queries to be sent to the server, even
Expand Down