From 5b12d3a58ee8eef8ec0f6fc0e31ac1f412a86af5 Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Sat, 24 Feb 2018 09:44:42 +0100 Subject: [PATCH] net: do not inherit the no-half-open enforcer `Socket.prototype.destroySoon()` is called as soon as `UV_EOF` is read if the `allowHalfOpen` option is disabled. This already works as a "no-half-open enforcer" so there is no need to inherit another from `stream.Duplex`. PR-URL: https://github.com/nodejs/node/pull/18974 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Anna Henningsen Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Chen Gang Reviewed-By: Ruben Bridgewater --- lib/net.js | 11 +++++++---- test/parallel/test-http-connect.js | 7 +------ test/parallel/test-net-socket-no-halfopen-enforcer.js | 11 +++++++++++ 3 files changed, 19 insertions(+), 10 deletions(-) create mode 100644 test/parallel/test-net-socket-no-halfopen-enforcer.js diff --git a/lib/net.js b/lib/net.js index 90e0db558e3a9b..17a128a4f0c8db 100644 --- a/lib/net.js +++ b/lib/net.js @@ -50,6 +50,7 @@ const { async_id_symbol } = process.binding('async_wrap'); const { newUid, defaultTriggerAsyncIdScope } = require('internal/async_hooks'); const { nextTick } = require('internal/process/next_tick'); const errors = require('internal/errors'); +const DuplexBase = require('internal/streams/duplex_base'); const dns = require('dns'); const kLastWriteQueueSize = Symbol('lastWriteQueueSize'); @@ -211,7 +212,11 @@ function Socket(options) { else if (options === undefined) options = {}; - stream.Duplex.call(this, options); + // `DuplexBase` is just a slimmed down constructor for `Duplex` which allow + // us to not inherit the "no-half-open enforcer" as there is already one in + // place. Instances of `Socket` are still instances of `Duplex`, that is, + // `socket instanceof Duplex === true`. + DuplexBase.call(this, options); if (options.handle) { this._handle = options.handle; // private @@ -236,8 +241,6 @@ function Socket(options) { this._writev = null; this._write = makeSyncWrite(fd); } - this.readable = options.readable !== false; - this.writable = options.writable !== false; } else { // these will be set once there is a connection this.readable = this.writable = false; @@ -256,7 +259,7 @@ function Socket(options) { this._writableState.decodeStrings = false; // default to *not* allowing half open sockets - this.allowHalfOpen = options && options.allowHalfOpen || false; + this.allowHalfOpen = options.allowHalfOpen || false; // if we have a handle, then start the flow of data into the // buffer. if not, then this will happen when we connect diff --git a/test/parallel/test-http-connect.js b/test/parallel/test-http-connect.js index 9b7432f03a7542..af0948dd067054 100644 --- a/test/parallel/test-http-connect.js +++ b/test/parallel/test-http-connect.js @@ -68,12 +68,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-net-socket-no-halfopen-enforcer.js b/test/parallel/test-net-socket-no-halfopen-enforcer.js new file mode 100644 index 00000000000000..3df5b6d7b9c8cb --- /dev/null +++ b/test/parallel/test-net-socket-no-halfopen-enforcer.js @@ -0,0 +1,11 @@ +'use strict'; +require('../common'); + +// This test ensures that `net.Socket` does not inherit the no-half-open +// enforcer from `stream.Duplex`. + +const { Socket } = require('net'); +const { strictEqual } = require('assert'); + +const socket = new Socket({ allowHalfOpen: false }); +strictEqual(socket.listenerCount('end'), 1);