From efefdd668dece956c4f75039c77fb0c40dfdd3c8 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sat, 15 Feb 2020 01:56:01 +0100 Subject: [PATCH] net: autoDestroy Socket Refactors net.Socket into using autoDestroy functionality of streams. PR-URL: https://github.com/nodejs/node/pull/31806 Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Denys Otrishko Reviewed-By: Benjamin Gruenbaum --- lib/net.js | 36 +++++-------------- .../parallel/test-http2-max-invalid-frames.js | 5 ++- test/parallel/test-net-write-after-close.js | 11 +++--- test/parallel/test-tls-getprotocol.js | 3 +- .../test-tls-streamwrap-buffersize.js | 5 ++- 5 files changed, 22 insertions(+), 38 deletions(-) diff --git a/lib/net.js b/lib/net.js index ee0ffebe0ec45e..1f105a179f2298 100644 --- a/lib/net.js +++ b/lib/net.js @@ -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); @@ -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 @@ -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; } @@ -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(); + } } diff --git a/test/parallel/test-http2-max-invalid-frames.js b/test/parallel/test-http2-max-invalid-frames.js index 597bd8e81197b5..671aa833593017 100644 --- a/test/parallel/test-http2-max-invalid-frames.js +++ b/test/parallel/test-http2-max-invalid-frames.js @@ -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'); diff --git a/test/parallel/test-net-write-after-close.js b/test/parallel/test-net-write-after-close.js index f17273f9417a2a..2647e3d0d97095 100644 --- a/test/parallel/test-net-write-after-close.js +++ b/test/parallel/test-net-write-after-close.js @@ -21,6 +21,7 @@ 'use strict'; const common = require('../common'); +const assert = require('assert'); const net = require('net'); @@ -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() { @@ -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(); }); diff --git a/test/parallel/test-tls-getprotocol.js b/test/parallel/test-tls-getprotocol.js index ec3642cbf74614..a76ff0f3442a97 100644 --- a/test/parallel/test-tls-getprotocol.js +++ b/test/parallel/test-tls-getprotocol.js @@ -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) diff --git a/test/parallel/test-tls-streamwrap-buffersize.js b/test/parallel/test-tls-streamwrap-buffersize.js index f65a3d6e76b9d8..984cc51e505183 100644 --- a/test/parallel/test-tls-streamwrap-buffersize.js +++ b/test/parallel/test-tls-streamwrap-buffersize.js @@ -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); }));