stream: add no-half-open enforcer only if needed

The listener does not do anything if `allowHalfOpen` is enabled. Add it only when `allowHalfOpen` is disabled. PR-URL: nodejs#18953 Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
lpinca · Mar 7, 2018 · 64746c7 · 64746c7
1 parent 23807d7
commit 64746c7
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 12 deletions.
diff --git a/lib/_stream_duplex.js b/lib/_stream_duplex.js
@@ -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', {
@@ -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.

diff --git a/test/parallel/test-http-connect.js b/test/parallel/test-http-connect.js
@@ -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();
@@ -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);

diff --git a/test/parallel/test-stream-duplex.js b/test/parallel/test-stream-duplex.js
@@ -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;