From 694b4422ef9b7f0f269f8ccd3bfe43612e15637b Mon Sep 17 00:00:00 2001 From: Jessica Lord Date: Mon, 11 Dec 2017 19:26:26 -0500 Subject: [PATCH 1/4] fix(mongos): in topologyClosed event remove all listeners --- lib/topologies/mongos.js | 5 +++ test/tests/unit/mongos/events_tests.js | 49 ++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 test/tests/unit/mongos/events_tests.js diff --git a/lib/topologies/mongos.js b/lib/topologies/mongos.js index 842520622..51c7adc7f 100644 --- a/lib/topologies/mongos.js +++ b/lib/topologies/mongos.js @@ -273,6 +273,11 @@ function emitSDAMEvent(self, event, description) { if (self.listeners(event).length > 0) { self.emit(event, description); } + if (event === 'topologyClosed') { + if (self.disconnectedProxies) { + self.disconnectedProxies.forEach(p => { p.removeAllListeners(); } ) + } + } } /** diff --git a/test/tests/unit/mongos/events_tests.js b/test/tests/unit/mongos/events_tests.js new file mode 100644 index 000000000..ed6577f7b --- /dev/null +++ b/test/tests/unit/mongos/events_tests.js @@ -0,0 +1,49 @@ +'use strict'; + +const expect = require('chai').expect; +const Mongos = require('../../../../lib/topologies/mongos'); +const mock = require('../../../mock'); +const MongosFixture = require('../common').MongosFixture; + +const test = new MongosFixture(); + +describe.only('EventEmitters (Mongos)', function() { + afterEach(() => mock.cleanup()); + beforeEach(() => { + return mock.createServer().then(mockServer => { + test.server = mockServer; + }); + }); + + it('should remove all event listeners when server is closed', { + metadata: { requires: { topology: ['single'] } }, + test: function(done) { + test.server.setMessageHandler(req => { + const doc = req.document; + if (doc.ismaster) { + req.reply(Object.assign({}, test.defaultFields)); + } + }); + + const mongos = new Mongos([test.server.address()], { + connectionTimeout: 30000, + socketTimeout: 30000, + haInterval: 500, + size: 1 + }); + + mongos.on('error', done); + mongos.once('connect', () => { + // After we connect, destroy/close the server + mongos.destroy(); + mongos.disconnectedProxies.forEach(p => { + // There should be no listeners remaining + expect(p.listenerCount()).to.equal(0); + }); + done(); + }); + + mongos.connect(); + } + }); +}); From 9fc3d1d241ad739201aa920377a82e5bd8fa9bdc Mon Sep 17 00:00:00 2001 From: Jessica Lord Date: Mon, 11 Dec 2017 19:30:37 -0500 Subject: [PATCH 2/4] fix(mongos): remove all listeners on destroy --- lib/topologies/mongos.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/topologies/mongos.js b/lib/topologies/mongos.js index 51c7adc7f..530d3d91f 100644 --- a/lib/topologies/mongos.js +++ b/lib/topologies/mongos.js @@ -273,11 +273,6 @@ function emitSDAMEvent(self, event, description) { if (self.listeners(event).length > 0) { self.emit(event, description); } - if (event === 'topologyClosed') { - if (self.disconnectedProxies) { - self.disconnectedProxies.forEach(p => { p.removeAllListeners(); } ) - } - } } /** @@ -856,7 +851,10 @@ Mongos.prototype.destroy = function(options) { // Move to list of disconnectedProxies moveServerFrom(self.connectedProxies, self.disconnectedProxies, x); }); - + // Remove all listeners + self.disconnectedProxies.forEach(p => { + p.removeAllListeners(); + }); // Emit the final topology change emitTopologyDescriptionChanged(self); // Emit toplogy closing event From a638aef0151bd0688fd2ceb5f168070a2231cf1a Mon Sep 17 00:00:00 2001 From: Jessica Lord Date: Tue, 12 Dec 2017 16:22:24 -0500 Subject: [PATCH 3/4] fix(mongos): remove listener on destroy event after servers are created --- lib/topologies/mongos.js | 15 ++++++++------- test/tests/unit/mongos/events_tests.js | 13 +++++++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/lib/topologies/mongos.js b/lib/topologies/mongos.js index 530d3d91f..ab5e0fb71 100644 --- a/lib/topologies/mongos.js +++ b/lib/topologies/mongos.js @@ -299,10 +299,15 @@ Mongos.prototype.connect = function(options) { ); }); + const serverDescriptionChangedCallback = event => { + self.emit('serverDescriptionChanged', event); + }; + servers.forEach(function(server) { - server.on('serverDescriptionChanged', function(event) { - self.emit('serverDescriptionChanged', event); - }); + server.on('serverDescriptionChanged', serverDescriptionChangedCallback); + server.on('destroy', () => + server.removeListener('serverDescriptionChanged', serverDescriptionChangedCallback) + ); }); // Emit the topology opening event @@ -851,10 +856,6 @@ Mongos.prototype.destroy = function(options) { // Move to list of disconnectedProxies moveServerFrom(self.connectedProxies, self.disconnectedProxies, x); }); - // Remove all listeners - self.disconnectedProxies.forEach(p => { - p.removeAllListeners(); - }); // Emit the final topology change emitTopologyDescriptionChanged(self); // Emit toplogy closing event diff --git a/test/tests/unit/mongos/events_tests.js b/test/tests/unit/mongos/events_tests.js index ed6577f7b..8b24f3e04 100644 --- a/test/tests/unit/mongos/events_tests.js +++ b/test/tests/unit/mongos/events_tests.js @@ -15,7 +15,7 @@ describe.only('EventEmitters (Mongos)', function() { }); }); - it('should remove all event listeners when server is closed', { + it('should remove `serverDescriptionChanged` listeners when server is closed', { metadata: { requires: { topology: ['single'] } }, test: function(done) { test.server.setMessageHandler(req => { @@ -34,12 +34,17 @@ describe.only('EventEmitters (Mongos)', function() { mongos.on('error', done); mongos.once('connect', () => { + expect(mongos.disconnectedProxies).to.have.length(1); + expect(mongos.disconnectedProxies[0].listenerCount('serverDescriptionChanged')).to.equal(1); // After we connect, destroy/close the server mongos.destroy(); - mongos.disconnectedProxies.forEach(p => { - // There should be no listeners remaining - expect(p.listenerCount()).to.equal(0); + mongos.on('topologyClosed', () => { + expect(mongos.disconnectedProxies).to.have.length(1); + expect(mongos.disconnectedProxies[0].listenerCount('serverDescriptionChanged')).to.equal( + 0 + ); }); + done(); }); From eb427b34ca1438d23185d53022c8df04d6e901d5 Mon Sep 17 00:00:00 2001 From: Jessica Lord Date: Tue, 12 Dec 2017 16:23:18 -0500 Subject: [PATCH 4/4] fix(test): remove only --- test/tests/unit/mongos/events_tests.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tests/unit/mongos/events_tests.js b/test/tests/unit/mongos/events_tests.js index 8b24f3e04..1e1fc2c0b 100644 --- a/test/tests/unit/mongos/events_tests.js +++ b/test/tests/unit/mongos/events_tests.js @@ -7,7 +7,7 @@ const MongosFixture = require('../common').MongosFixture; const test = new MongosFixture(); -describe.only('EventEmitters (Mongos)', function() { +describe('EventEmitters (Mongos)', function() { afterEach(() => mock.cleanup()); beforeEach(() => { return mock.createServer().then(mockServer => {