Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
17 changes: 16 additions & 1 deletion lib/drivers/node-mongodb-native/collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ function iter(i) {

let _args = args;
let callback = null;
let timeout = null;
if (this._shouldBufferCommands() && this.buffer) {
this.conn.emit('buffer', {
_id: opId,
Expand All @@ -136,7 +137,6 @@ function iter(i) {
let callback;
let _args = args;
let promise = null;
let timeout = null;
if (syncCollectionMethods[i] && typeof lastArg === 'function') {
this.addQueue(i, _args);
callback = lastArg;
Expand Down Expand Up @@ -239,6 +239,9 @@ function iter(i) {

if (syncCollectionMethods[i] && typeof lastArg === 'function') {
const result = collection[i].apply(collection, _args.slice(0, _args.length - 1));
if (timeout != null) {
clearTimeout(timeout);
}
this.conn.emit('operation-end', { _id: opId, modelName: _this.modelName, collectionName: this.name, method: i, result });
return lastArg.call(this, null, result);
}
Expand All @@ -247,6 +250,9 @@ function iter(i) {
if (ret != null && typeof ret.then === 'function') {
return ret.then(
result => {
if (timeout != null) {
clearTimeout(timeout);
}
if (typeof lastArg === 'function') {
lastArg(null, result);
} else {
Expand All @@ -255,6 +261,9 @@ function iter(i) {
return result;
},
error => {
if (timeout != null) {
clearTimeout(timeout);
}
if (typeof lastArg === 'function') {
lastArg(error);
return;
Expand All @@ -265,10 +274,16 @@ function iter(i) {
}
);
}
if (timeout != null) {
clearTimeout(timeout);
}
return ret;
} catch (error) {
// Collection operation may throw because of max bson size, catch it here
// See gh-3906
if (timeout != null) {
clearTimeout(timeout);
}
if (typeof lastArg === 'function') {
return lastArg(error);
} else {
Expand Down
45 changes: 25 additions & 20 deletions lib/drivers/node-mongodb-native/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,13 @@ NativeConnection.prototype.useDb = function(name, options) {

newConn.name = name;

// push onto the otherDbs stack, this is used when state changes
this.otherDbs.push(newConn);
newConn.otherDbs.push(this);

// push onto the relatedDbs cache, this is used when state changes
if (options && options.useCache) {
this.relatedDbs[newConn.name] = newConn;
newConn.relatedDbs = this.relatedDbs;
// Add to relatedDbs for state synchronization and caching
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undo this change

this.relatedDbs[newConn.name] = newConn;
newConn.relatedDbs = this.relatedDbs;

// Ensure the parent connection is also tracked in the new connection's relatedDbs
if (this.name && !newConn.relatedDbs[this.name]) {
newConn.relatedDbs[this.name] = this;
}

return newConn;
Expand Down Expand Up @@ -160,19 +159,21 @@ NativeConnection.prototype.aggregate = function aggregate(pipeline, options) {
*/

NativeConnection.prototype.removeDb = function removeDb(name) {
const dbs = this.otherDbs.filter(db => db.name === name);
if (!dbs.length) {
const db = this.relatedDbs[name];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undo this change

if (!db) {
throw new MongooseError(`No connections to database "${name}" found`);
}

for (const db of dbs) {
db._closeCalled = true;
db._destroyCalled = true;
db._readyState = STATES.disconnected;
db.$wasForceClosed = true;
db._closeCalled = true;
db._destroyCalled = true;
db._readyState = STATES.disconnected;
db.$wasForceClosed = true;

// Remove from all related connections' relatedDbs
for (const relatedDb of Object.values(this.relatedDbs)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undo this change

delete relatedDb.relatedDbs[name];
}
delete this.relatedDbs[name];
this.otherDbs = this.otherDbs.filter(db => db.name !== name);
};

/**
Expand Down Expand Up @@ -345,8 +346,10 @@ NativeConnection.prototype.createClient = async function createClient(uri, optio

_setClient(this, client, options, dbName);

for (const db of this.otherDbs) {
_setClient(db, client, {}, db.name);
for (const db of Object.values(this.relatedDbs)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undo this change

if (db !== this) {
_setClient(db, client, {}, db.name);
}
}
return this;
};
Expand Down Expand Up @@ -494,8 +497,10 @@ function _setClient(conn, client, options, dbName) {

client.on('serverHeartbeatSucceeded', () => {
conn._lastHeartbeatAt = Date.now();
for (const otherDb of conn.otherDbs) {
otherDb._lastHeartbeatAt = conn._lastHeartbeatAt;
for (const otherDb of Object.values(conn.relatedDbs)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undo this change

if (otherDb !== conn) {
otherDb._lastHeartbeatAt = conn._lastHeartbeatAt;
}
}
});

Expand Down
57 changes: 57 additions & 0 deletions test/collection.timeout.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
'use strict';

const assert = require('assert');
const mongoose = require('../index');

describe('Collection timeout cleanup', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move these tests into collection.test.js

let db;

before(async function() {
db = await mongoose.createConnection('mongodb://127.0.0.1:27017/mongoose_test').asPromise();
});

after(async function() {
await db.close();
});

it('should clear timeout on successful operations', async function() {
const TestModel = db.model('Test', { name: String });

// Track active handles before operations
const initialHandles = process._getActiveHandles().length;

// Execute multiple operations that should complete successfully
for (let i = 0; i < 10; i++) {
await TestModel.find({}).exec();
}

// Allow some time for any lingering timeouts
await new Promise(resolve => setTimeout(resolve, 100));

// Active handles should not have grown significantly
const finalHandles = process._getActiveHandles().length;
assert.ok(finalHandles <= initialHandles + 2,
`Expected handles to remain stable, but grew from ${initialHandles} to ${finalHandles}`);
});

it('should clear timeout on operation errors', async function() {
const TestModel = db.model('Test2', { name: String });

const initialHandles = process._getActiveHandles().length;

// Execute operations that will fail
for (let i = 0; i < 5; i++) {
try {
await TestModel.collection.findOne({ $invalidOperator: true });
} catch (err) {
// Expected to fail
}
}

await new Promise(resolve => setTimeout(resolve, 100));

const finalHandles = process._getActiveHandles().length;
assert.ok(finalHandles <= initialHandles + 2,
`Expected handles to remain stable after errors, but grew from ${initialHandles} to ${finalHandles}`);
});
});