Skip to content

Commit

Permalink
net: do not inherit the no-half-open enforcer
Browse files Browse the repository at this point in the history
`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: #18974
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Chen Gang <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
  • Loading branch information
lpinca authored and MylesBorins committed Mar 20, 2018
1 parent 152c931 commit 5b12d3a
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 10 deletions.
11 changes: 7 additions & 4 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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
Expand Down
7 changes: 1 addition & 6 deletions test/parallel/test-http-connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 11 additions & 0 deletions test/parallel/test-net-socket-no-halfopen-enforcer.js
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit 5b12d3a

Please sign in to comment.