diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index f1493d274b..77b78afe9c 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -477,6 +477,16 @@ OutgoingMessage.prototype.end = function(data, encoding, callback) { if (this.finished) { return false; } + + var self = this; + function finish() { + self.emit('finish'); + } + + if (util.isFunction(callback)) + this.once('finish', callback); + + if (!this._header) { this._implicitHeader(); } @@ -497,10 +507,10 @@ OutgoingMessage.prototype.end = function(data, encoding, callback) { } if (this.chunkedEncoding) { - ret = this._send('0\r\n' + this._trailer + '\r\n', 'ascii', callback); + ret = this._send('0\r\n' + this._trailer + '\r\n', 'ascii', finish); } else { // Force a flush, HACK. - ret = this._send('', 'ascii', callback); + ret = this._send('', 'ascii', finish); } if (this.connection && data) @@ -521,7 +531,7 @@ OutgoingMessage.prototype.end = function(data, encoding, callback) { OutgoingMessage.prototype._finish = function() { assert(this.connection); - this.emit('finish'); + this.emit('prefinish'); }; diff --git a/lib/_http_server.js b/lib/_http_server.js index 2148487b5d..15ef5a6387 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -429,7 +429,7 @@ function connectionListener(socket) { // When we're finished writing the response, check if this is the last // respose, if so destroy the socket. - res.on('finish', resOnFinish); + res.on('prefinish', resOnFinish); function resOnFinish() { // Usually the first incoming element should be our request. it may // be that in the case abortIncoming() was called that the incoming diff --git a/test/simple/test-http-byteswritten.js b/test/simple/test-http-byteswritten.js index ff842414cf..92bb2729c5 100644 --- a/test/simple/test-http-byteswritten.js +++ b/test/simple/test-http-byteswritten.js @@ -26,12 +26,19 @@ var http = require('http'); var body = 'hello world\n'; +var sawFinish = false; +process.on('exit', function() { + assert(sawFinish); + console.log('ok'); +}); + var httpServer = http.createServer(function(req, res) { + httpServer.close(); + res.on('finish', function() { + sawFinish = true; assert(typeof(req.connection.bytesWritten) === 'number'); assert(req.connection.bytesWritten > 0); - httpServer.close(); - console.log('ok'); }); res.writeHead(200, { 'Content-Type': 'text/plain' }); @@ -51,6 +58,9 @@ var httpServer = http.createServer(function(req, res) { }); httpServer.listen(common.PORT, function() { - http.get({ port: common.PORT }); + // XXX(isaacs): This should not be necessary. + http.get({ port: common.PORT }, function(res) { + res.resume(); + }); }); diff --git a/test/simple/test-http-outgoing-finish.js b/test/simple/test-http-outgoing-finish.js new file mode 100644 index 0000000000..7acea07c99 --- /dev/null +++ b/test/simple/test-http-outgoing-finish.js @@ -0,0 +1,74 @@ +// 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'); + +http.createServer(function(req, res) { + write(res); + req.resume(); + this.close(); +}).listen(common.PORT, function() { + var req = http.request({ + port: common.PORT, + method: 'PUT' + }); + write(req); + req.on('response', function(res) { + res.resume(); + }); +}); + +var buf = new Buffer(1024 * 16); +buf.fill('x'); +function write(out) { + var name = out.constructor.name; + var finishEvent = false; + var endCb = false; + + // first, write until it gets some backpressure + while (out.write(buf)) {} + + // now end, and make sure that we don't get the 'finish' event + // before the tick where the cb gets called. We give it until + // nextTick because this is added as a listener before the endcb + // is registered. The order is not what we're testing here, just + // that 'finish' isn't emitted until the stream is fully flushed. + out.on('finish', function() { + finishEvent = true; + console.error('%s finish event', name); + process.nextTick(function() { + assert(endCb, name + ' got finish event before endcb!'); + console.log('ok - %s finishEvent', name); + }); + }); + + out.end(buf, function() { + endCb = true; + console.error('%s endCb', name); + process.nextTick(function() { + assert(endCb, name + ' got endCb event before finishEvent!'); + console.log('ok - %s endCb', name); + }); + }); +}