diff --git a/lib/http.js b/lib/http.js index f5418b00f7c..aee579aadf4 100644 --- a/lib/http.js +++ b/lib/http.js @@ -967,7 +967,24 @@ exports.ServerResponse = ServerResponse; ServerResponse.prototype.statusCode = 200; function onServerResponseClose() { - this._httpMessage.emit('close'); + // EventEmitter.emit makes a copy of the 'close' listeners array before + // calling the listeners. detachSocket() unregisters onServerResponseClose + // but if detachSocket() is called, directly or indirectly, by a 'close' + // listener, onServerResponseClose is still in that copy of the listeners + // array. That is, in the example below, b still gets called even though + // it's been removed by a: + // + // var obj = new events.EventEmitter; + // obj.on('event', a); + // obj.on('event', b); + // function a() { obj.removeListener('event', b) } + // function b() { throw "BAM!" } + // obj.emit('event'); // throws + // + // Ergo, we need to deal with stale 'close' events and handle the case + // where the ServerResponse object has already been deconstructed. + // Fortunately, that requires only a single if check. :-) + if (this._httpMessage) this._httpMessage.emit('close'); } ServerResponse.prototype.assignSocket = function(socket) { diff --git a/test/simple/test-http-server-stale-close.js b/test/simple/test-http-server-stale-close.js new file mode 100644 index 00000000000..e8ecf318e4c --- /dev/null +++ b/test/simple/test-http-server-stale-close.js @@ -0,0 +1,51 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +var common = require('../common'); +var assert = require('assert'); +var http = require('http'); +var fork = require('child_process').fork; + +if (process.env.NODE_TEST_FORK) { + var req = http.request({ + headers: {'Content-Length': '42'}, + method: 'POST', + host: '127.0.0.1', + port: common.PORT, + }, process.exit); + req.write('BAM'); + req.end(); +} +else { + var server = http.createServer(function(req, res) { + res.writeHead(200, {'Content-Length': '42'}); + req.pipe(res); + req.on('close', function() { + server.close(); + res.end(); + }); + }); + server.listen(common.PORT, function() { + fork(__filename, { + env: {NODE_TEST_FORK: '1'} + }); + }); +}