Skip to content

Commit

Permalink
stream: add no-half-open enforcer only if needed
Browse files Browse the repository at this point in the history
The listener does not do anything if `allowHalfOpen` is enabled. Add it
only when `allowHalfOpen` is disabled.

PR-URL: #18953
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
lpinca committed Mar 7, 2018
1 parent 23807d7 commit 64746c7
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 12 deletions.
11 changes: 5 additions & 6 deletions lib/_stream_duplex.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ function Duplex(options) {
this.writable = false;

this.allowHalfOpen = true;
if (options && options.allowHalfOpen === false)
if (options && options.allowHalfOpen === false) {
this.allowHalfOpen = false;

this.once('end', onend);
this.once('end', onend);
}
}

Object.defineProperty(Duplex.prototype, 'writableHighWaterMark', {
Expand Down Expand Up @@ -96,9 +96,8 @@ Object.defineProperty(Duplex.prototype, 'writableLength', {

// the no-half-open enforcer
function onend() {
// if we allow half-open state, or if the writable side ended,
// then we're ok.
if (this.allowHalfOpen || this._writableState.ended)
// If the writable side ended, then we're ok.
if (this._writableState.ended)
return;

// no more data can be written.
Expand Down
14 changes: 8 additions & 6 deletions test/parallel/test-http-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ server.on('connect', common.mustCall((req, socket, firstBodyChunk) => {
assert.strictEqual(req.method, 'CONNECT');
assert.strictEqual(req.url, 'google.com:443');

// Make sure this socket has detached.
assert.strictEqual(socket.listeners('close').length, 0);
assert.strictEqual(socket.listeners('drain').length, 0);
assert.strictEqual(socket.listeners('data').length, 0);
assert.strictEqual(socket.listeners('end').length, 1);
assert.strictEqual(socket.listeners('error').length, 1);

socket.write('HTTP/1.1 200 Connection established\r\n\r\n');

let data = firstBodyChunk.toString();
Expand Down Expand Up @@ -68,12 +75,7 @@ server.listen(0, common.mustCall(() => {
assert.strictEqual(socket.listeners('connect').length, 0);
assert.strictEqual(socket.listeners('data').length, 0);
assert.strictEqual(socket.listeners('drain').length, 0);

// the stream.Duplex onend listener
// allow 0 here, so that i can run the same test on streams1 impl
assert(socket.listenerCount('end') <= 2,
`Found ${socket.listenerCount('end')} end listeners`);

assert.strictEqual(socket.listeners('end').length, 1);
assert.strictEqual(socket.listeners('free').length, 0);
assert.strictEqual(socket.listeners('close').length, 0);
assert.strictEqual(socket.listeners('error').length, 0);
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-stream-duplex.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ const stream = new Duplex({ objectMode: true });
assert(Duplex() instanceof Duplex);
assert(stream._readableState.objectMode);
assert(stream._writableState.objectMode);
assert(stream.allowHalfOpen);
assert.strictEqual(stream.listenerCount('end'), 0);

let written;
let read;
Expand Down

0 comments on commit 64746c7

Please sign in to comment.