Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Cleaning up server event listeners in WebSocketServer close #253

Open
wants to merge 2 commits into from

1 participant

Rob Gilson
Rob Gilson

This was causing subsequent websockets on the same url to fail to upgrade properly for me.

There is still another, unrelated issue related to repeatedly creating and destroying websockets on the same URL. Because this could get confusing quickly I'm going to note in big bold letters here: This Patch Does Not Fix Issue #241.

Running the included patched test against the current eianros:master should fail whereas running it against this branch should succeed.

In the immortal words of Douglas Adams: "Share and Enjoy"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 43 additions and 16 deletions.
  1. +40 −16 lib/WebSocketServer.js
  2. +3 −0  test/WebSocketServer.test.js
56 lib/WebSocketServer.js
View
@@ -37,6 +37,31 @@ function WebSocketServer(options, callback) {
var self = this;
+ self._onServerListening = function() { self.emit('listening'); };
+
+ /**
+ * Curry HTTP Server Events
+ */
+
+ self._onServerError = function(error) {
+ self.emit('error', error)
+ };
+
+ /**
+ * Handle upgrade events from the HTTP Server
+ */
+
+ self._onServerUpgrade = function(req, socket, upgradeHead) {
+ //copy upgradeHead to avoid retention of large slab buffers used in node core
+ var head = new Buffer(upgradeHead.length);
+ upgradeHead.copy(head);
+
+ self.handleUpgrade(req, socket, head, function(client) {
+ self.emit('connection'+req.url, client);
+ self.emit('connection', client);
+ });
+ };
+
if (options.isDefinedAndNonNull('port')) {
this._server = http.createServer(function (req, res) {
res.writeHead(200, {'Content-Type': 'text/plain'});
@@ -59,22 +84,12 @@ function WebSocketServer(options, callback) {
this._server._webSocketPaths[options.value.path] = 1;
}
}
- if (this._server) this._server.once('listening', function() { self.emit('listening'); });
+ if (this._server) this._server.once('listening', this._onServerListening);
if (typeof this._server != 'undefined') {
- this._server.on('error', function(error) {
- self.emit('error', error)
- });
- this._server.on('upgrade', function(req, socket, upgradeHead) {
- //copy upgradeHead to avoid retention of large slab buffers used in node core
- var head = new Buffer(upgradeHead.length);
- upgradeHead.copy(head);
-
- self.handleUpgrade(req, socket, head, function(client) {
- self.emit('connection'+req.url, client);
- self.emit('connection', client);
- });
- });
+ this._server
+ .on('error', this._onServerError)
+ .on('upgrade', this._onServerUpgrade);
}
this.options = options.value;
@@ -95,6 +110,15 @@ util.inherits(WebSocketServer, events.EventEmitter);
*/
WebSocketServer.prototype.close = function() {
+ // Remove the http server event listeners
+ if (this._server != undefined)
+ {
+ this._server.removeListener('listening', this._onServerListening);
+ this._server.removeListener('error', this._onServerError);
+ this._server.removeListener('upgrade', this._onServerUpgrade);
+ }
+
+
// terminate all associated clients
var error = null;
try {
@@ -107,7 +131,7 @@ WebSocketServer.prototype.close = function() {
}
// remove path descriptor, if any
- if (this.path && this._server._webSocketPaths) {
+ if (this.path && this._server != undefined && this._server._webSocketPaths) {
delete this._server._webSocketPaths[this.path];
if (Object.keys(this._server._webSocketPaths).length == 0) {
delete this._server._webSocketPaths;
@@ -457,4 +481,4 @@ function abortConnection(socket, code, name) {
// ensure that an early aborted connection is shut down completely
try { socket.destroy(); } catch (e) {}
}
-}
+}
3  test/WebSocketServer.test.js
View
@@ -204,6 +204,9 @@ describe('WebSocketServer', function() {
Object.keys(srv._webSocketPaths).length.should.eql(1);
wss2.close();
(typeof srv._webSocketPaths).should.eql('undefined');
+ (typeof srv._events.listening).should.eql('undefined');
+ (typeof srv._events.error).should.eql('undefined');
+ (typeof srv._events.upgrade).should.eql('undefined');
srv.close();
done();
});
Something went wrong with that request. Please try again.