Skip to content
This repository has been archived by the owner on Feb 4, 2022. It is now read-only.

Commit

Permalink
fix(pool): clean up connections if pool is destroyed mid-handshake
Browse files Browse the repository at this point in the history
Fixes NODE-1971
  • Loading branch information
daprahamian committed May 30, 2019
1 parent 853bcfe commit 4bd7f1c
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 0 deletions.
1 change: 1 addition & 0 deletions lib/connection/pool.js
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,7 @@ Pool.prototype.connect = function() {
}

if (self.state === DESTROYED || self.state === DESTROYING) {
connection.destroy();
return self.destroy();
}

Expand Down
61 changes: 61 additions & 0 deletions test/tests/unit/pool_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const mock = require('mongodb-mock-server');
const Server = require('../../../lib/topologies/server');
const MongoWriteConcernError = require('../../../lib/error').MongoWriteConcernError;
const sinon = require('sinon');
const Socket = require('net').Socket;

const test = {};
describe('Pool (unit)', function() {
Expand Down Expand Up @@ -77,4 +78,64 @@ describe('Pool (unit)', function() {

client.connect();
});

it('should make sure to close connection if destroy is called mid-handshake', function(done) {
class Deferred {
constructor() {
this.promise = new Promise((resolve, reject) => {
this.resolve = resolve;
this.reject = reject;
});
}
}

function getActiveSockets() {
return new Set(process._getActiveHandles().filter(handle => handle instanceof Socket));
}

function diffSet(base, sub) {
const ret = new Set();
for (const item of base) {
if (!sub.has(item)) {
ret.add(item);
}
}

return ret;
}

const requestReceived = new Deferred();
const sendReply = new Deferred();

test.server.setMessageHandler(request => {
requestReceived.resolve();
const doc = request.document;
if (doc.ismaster) {
sendReply.promise.then(() => {
request.reply(Object.assign({}, mock.DEFAULT_ISMASTER));
});
}
});

const client = new Server(test.server.address());

const previouslyActiveSockets = getActiveSockets();
client.connect();

requestReceived.promise.then(() => {
client.destroy({}, () => {
sendReply.resolve();
setTimeout(() => {
const activeSockets = diffSet(getActiveSockets(), previouslyActiveSockets);
try {
expect(activeSockets.size).to.equal(0);
done();
} catch (e) {
console.dir(activeSockets, { depth: 0 });
done(e);
}
}, 50);
});
});
});
});

1 comment on commit 4bd7f1c

@DonMartin76
Copy link

Choose a reason for hiding this comment

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

I can confirm that this fix certainly has a positive effect on the mongodb node.js driver. We have been chasing a connection leak like crazy, but it turned out it was since we updated to 3.2.6 it occurred. With 3.2.7, everything is back to normal.

Please sign in to comment.