net: autoDestroy Socket

Refactors net.Socket into using autoDestroy functionality of streams. PR-URL: #31806 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Denys Otrishko <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
nodejs · Apr 2, 2020 · efefdd6 · efefdd6
1 parent 882b61a
commit efefdd6
Show file tree

Hide file tree

Showing 5 changed files with 22 additions and 38 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);
         }));