From 192b28bf2fc688f3da9ad0caa0ecbb84efaf3330 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20=C5=A0rom?= Date: Fri, 25 Sep 2015 22:40:59 +0200 Subject: [PATCH] HTTP outgoing queue bug fix (#2639) This change fix bug of incorrect http outgoing queue which caused crash on assert. I think there should be some other changes to make ending of response more transparent, this change only fix the bug. --- lib/_http_outgoing.js | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index ddbd6c3828aaa8..a2096c5758569a 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -51,7 +51,8 @@ function OutgoingMessage() { this.outputCallbacks = []; this.writable = true; - + this.explicitfinish = false; + this._last = false; this.chunkedEncoding = false; this.shouldKeepAlive = true; @@ -133,8 +134,12 @@ OutgoingMessage.prototype._writeRaw = function(data, encoding, callback) { encoding = null; } + //if this happens and callback is finish - it will be fired in bad time!!! + assert(!(this.connection && data.length === 0 && this.output.length > 0)); + if (data.length === 0) { - if (typeof callback === 'function') + //callback cannot be called (finish) when the connection is not assigned + if (typeof callback === 'function' && this.connection) process.nextTick(callback); return true; } @@ -536,7 +541,9 @@ OutgoingMessage.prototype.end = function(data, encoding, callback) { var self = this; function finish() { - self.emit('finish'); + //finish MUST NOT be called when connection is not assigned + assert(self.connection); + self.emit('finish'); } if (typeof callback === 'function') @@ -574,6 +581,12 @@ OutgoingMessage.prototype.end = function(data, encoding, callback) { } else { // Force a flush, HACK. ret = this._send('', 'binary', finish); + if (!this.connection) { + //if there is nothing to send and connection is not assigned, nothing is put into output buffer, so no callback will be called after write + //and nexttick callback in _writeraw MUST be skipped - so finish must be explicitly called after flushing this message as it becomes the first in queue + //this should be really rewritten + this.explicitfinish = true; + } } if (this.connection && data) @@ -587,14 +600,14 @@ OutgoingMessage.prototype.end = function(data, encoding, callback) { if (this.output.length === 0 && this.connection && this.connection._httpMessage === this) { - this._finish(); + this.prefinish(); } return ret; }; -OutgoingMessage.prototype._finish = function() { +OutgoingMessage.prototype.prefinish = function() { assert(this.connection); this.emit('prefinish'); }; @@ -642,7 +655,14 @@ OutgoingMessage.prototype._flush = function() { if (this.finished) { // This is a queue to the server or client to bring in the next this. - this._finish(); + this.prefinish(); + if (this.explicitfinish) { + //finish MUST NOT be called when connection is not assigned + //message was already finished and finish callback was not in the queue + assert(this.connection); + this.emit('finish'); + } + } else if (ret) { // This is necessary to prevent https from breaking this.emit('drain');