Skip to content

Commit

Permalink
net: autoDestroy Socket
Browse files Browse the repository at this point in the history
Refactors net.Socket into using autoDestroy functionality
of streams.

PR-URL: #31806
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
ronag committed Apr 2, 2020
1 parent 882b61a commit efefdd6
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 38 deletions.
36 changes: 8 additions & 28 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,20 +285,15 @@ function Socket(options) {
else
options = { ...options };

const { allowHalfOpen } = options;

// Prevent the "no-half-open enforcer" from being inherited from `Duplex`.
options.allowHalfOpen = true;
// Default to *not* allowing half open sockets.
options.allowHalfOpen = Boolean(options.allowHalfOpen);
// For backwards compat do not emit close on destroy.
options.emitClose = false;
options.autoDestroy = false;
options.autoDestroy = true;
// Handle strings directly.
options.decodeStrings = false;
stream.Duplex.call(this, options);

// Default to *not* allowing half open sockets.
this.allowHalfOpen = Boolean(allowHalfOpen);

if (options.handle) {
this._handle = options.handle; // private
this[async_id_symbol] = getNewAsyncId(this._handle);
Expand Down Expand Up @@ -416,28 +411,18 @@ Socket.prototype._final = function(cb) {
const err = this._handle.shutdown(req);

if (err === 1 || err === UV_ENOTCONN) // synchronous finish
return afterShutdown.call(req, 0);
return cb();
else if (err !== 0)
return this.destroy(errnoException(err, 'shutdown'));
return cb(errnoException(err, 'shutdown'));
};


function afterShutdown(status) {
function afterShutdown() {
const self = this.handle[owner_symbol];

debug('afterShutdown destroyed=%j', self.destroyed,
self._readableState);

this.callback();

// Callback may come after call to destroy.
if (self.destroyed)
return;

if (!self.readable || self.readableEnded) {
debug('readableState ended, destroying');
self.destroy();
}
}

// Provide a better error message when we call end() as a result
Expand All @@ -452,10 +437,10 @@ function writeAfterFIN(chunk, encoding, cb) {
// eslint-disable-next-line no-restricted-syntax
const er = new Error('This socket has been ended by the other party');
er.code = 'EPIPE';
process.nextTick(emitErrorNT, this, er);
if (typeof cb === 'function') {
defaultTriggerAsyncIdScope(this[async_id_symbol], process.nextTick, cb, er);
}
this.destroy(er);

return false;
}
Expand Down Expand Up @@ -628,12 +613,7 @@ Socket.prototype.read = function(n) {
function onReadableStreamEnd() {
if (!this.allowHalfOpen) {
this.write = writeAfterFIN;
if (this.writable)
this.end();
else if (!this.writableLength)
this.destroy();
} else if (!this.destroyed && !this.writable && !this.writableLength)
this.destroy();
}
}


Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-http2-max-invalid-frames.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ server.on('stream', (stream) => {

server.listen(0, () => {
const h2header = Buffer.alloc(9);
const conn = net.connect(server.address().port);
const conn = net.connect({
port: server.address().port,
allowHalfOpen: true
});

conn.write('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n');

Expand Down
11 changes: 6 additions & 5 deletions test/parallel/test-net-write-after-close.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

'use strict';
const common = require('../common');
const assert = require('assert');

const net = require('net');

Expand All @@ -31,10 +32,7 @@ const server = net.createServer(common.mustCall(function(socket) {

socket.resume();

socket.on('error', common.mustCall(function(error) {
console.error('received error as expected, closing server', error);
server.close();
}));
socket.on('error', common.mustNotCall());
}));

server.listen(0, function() {
Expand All @@ -44,7 +42,10 @@ server.listen(0, function() {
// Then 'end' will be emitted when it receives a FIN packet from
// the other side.
client.on('end', common.mustCall(() => {
serverSocket.write('test', common.mustCall());
serverSocket.write('test', common.mustCall((err) => {
assert(err);
server.close();
}));
}));
client.end();
});
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-tls-getprotocol.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ const server = tls.createServer(serverConfig, common.mustCall(function() {
secureProtocol: v.secureProtocol
}, common.mustCall(function() {
assert.strictEqual(this.getProtocol(), v.version);
this.on('end', common.mustCall(function() {
this.on('end', common.mustCall());
this.on('close', common.mustCall(function() {
assert.strictEqual(this.getProtocol(), null);
})).end();
if (++connected === clientConfigs.length)
Expand Down
5 changes: 2 additions & 3 deletions test/parallel/test-tls-streamwrap-buffersize.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,8 @@ const net = require('net');
assert.strictEqual(client.bufferSize, i + 1);
}

// It seems that tlsSockets created from sockets of `Duplex` emit no
// "finish" events. We use "end" event instead.
client.on('end', common.mustCall(() => {
client.on('end', common.mustCall());
client.on('close', common.mustCall(() => {
assert.strictEqual(client.bufferSize, undefined);
}));

Expand Down

0 comments on commit efefdd6

Please sign in to comment.