Skip to content

Commit 0064848

Browse files
indutnyrvagg
authored andcommitted
http_server: pause socket properly
Account pending response data to decide whether pause the socket or not. Writable stream state is a not reliable measure, because it just says how much data is pending on a **current** request, thus not helping much with problem we are trying to solve here. PR-URL: nodejs#3128
1 parent eb0d901 commit 0064848

File tree

2 files changed

+27
-2
lines changed

2 files changed

+27
-2
lines changed

lib/_http_outgoing.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ function OutgoingMessage() {
4949
this.output = [];
5050
this.outputEncodings = [];
5151
this.outputCallbacks = [];
52+
this.outputSize = 0;
5253

5354
this.writable = true;
5455

@@ -71,6 +72,8 @@ function OutgoingMessage() {
7172
this._header = null;
7273
this._headers = null;
7374
this._headerNames = {};
75+
76+
this._onPendingData = null;
7477
}
7578
util.inherits(OutgoingMessage, Stream);
7679

@@ -120,6 +123,9 @@ OutgoingMessage.prototype._send = function(data, encoding, callback) {
120123
this.output.unshift(this._header);
121124
this.outputEncodings.unshift('binary');
122125
this.outputCallbacks.unshift(null);
126+
this.outputSize += this._header.length;
127+
if (this._onPendingData !== null)
128+
this._onPendingData(this._header.length);
123129
}
124130
this._headerSent = true;
125131
}
@@ -152,6 +158,9 @@ OutgoingMessage.prototype._writeRaw = function(data, encoding, callback) {
152158
this.output = [];
153159
this.outputEncodings = [];
154160
this.outputCallbacks = [];
161+
if (this._onPendingData !== null)
162+
this._onPendingData(-this.outputSize);
163+
this.outputSize = 0;
155164
} else if (data.length === 0) {
156165
if (typeof callback === 'function')
157166
process.nextTick(callback);
@@ -175,6 +184,9 @@ OutgoingMessage.prototype._buffer = function(data, encoding, callback) {
175184
this.output.push(data);
176185
this.outputEncodings.push(encoding);
177186
this.outputCallbacks.push(callback);
187+
this.outputSize += data.length;
188+
if (this._onPendingData !== null)
189+
this._onPendingData(data.length);
178190
return false;
179191
};
180192

lib/_http_server.js

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,8 @@ function Server(requestListener) {
240240
});
241241

242242
this.timeout = 2 * 60 * 1000;
243+
244+
this._pendingResponseData = 0;
243245
}
244246
util.inherits(Server, net.Server);
245247

@@ -259,6 +261,13 @@ function connectionListener(socket) {
259261
var self = this;
260262
var outgoing = [];
261263
var incoming = [];
264+
var outgoingData = 0;
265+
266+
function updateOutgoingData(delta) {
267+
outgoingData += delta;
268+
if (socket._paused && outgoingData < socket._writableState.highWaterMark)
269+
return socketOnDrain();
270+
}
262271

263272
function abortIncoming() {
264273
while (incoming.length) {
@@ -424,8 +433,10 @@ function connectionListener(socket) {
424433

425434
socket._paused = false;
426435
function socketOnDrain() {
436+
var needPause = outgoingData > socket._writableState.highWaterMark;
437+
427438
// If we previously paused, then start reading again.
428-
if (socket._paused) {
439+
if (socket._paused && !needPause) {
429440
socket._paused = false;
430441
socket.parser.resume();
431442
socket.resume();
@@ -439,7 +450,8 @@ function connectionListener(socket) {
439450
// so that we don't become overwhelmed by a flood of
440451
// pipelined requests that may never be resolved.
441452
if (!socket._paused) {
442-
var needPause = socket._writableState.needDrain;
453+
var needPause = socket._writableState.needDrain ||
454+
outgoingData >= socket._writableState.highWaterMark;
443455
if (needPause) {
444456
socket._paused = true;
445457
// We also need to pause the parser, but don't do that until after
@@ -450,6 +462,7 @@ function connectionListener(socket) {
450462
}
451463

452464
var res = new ServerResponse(req);
465+
res._onPendingData = updateOutgoingData;
453466

454467
res.shouldKeepAlive = shouldKeepAlive;
455468
DTRACE_HTTP_SERVER_REQUEST(req, socket);

0 commit comments

Comments
 (0)