net: emit 'close' after 'end'

Currently the writable side of the socket is closed as soon as `UV_EOF`
is read regardless of the state of the socket. This allows the handle
to be closed before `'end'` is emitted and thus `'close'` can be
emitted before `'end'` if the socket is paused.

This commit prevents the handle from being closed until `'end'` is
emitted ensuring the correct order of events.

PR-URL: https://github.com/nodejs/node/pull/19241
Fixes: https://github.com/nodejs/node/issues/19166
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>

1 files changed, 5 insertions, 12 deletions

diff --git a/lib/net.js b/lib/net.js
index 466531744e..f97b68d4c4 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -373,8 +373,6 @@ function afterShutdown(status, handle) {
   if (self._readableState.ended) {
     debug('readableState ended, destroying');
     self.destroy();
-  } else {
-    self.once('_socketEnd', self.destroy);
   }
 }
 
@@ -530,6 +528,11 @@ Socket.prototype.end = function(data, encoding, callback) {
 
 // Called when the 'end' event is emitted.
 function onReadableStreamEnd() {
+  if (!this.allowHalfOpen) {
+    this.write = writeAfterFIN;
+    if (this.writable)
+      this.end();
+  }
   maybeDestroy(this);
 }
 
@@ -649,16 +652,6 @@ function onread(nread, buffer) {
   // `end` -> `close`
   self.push(null);
   self.read(0);
-
-  if (!self.allowHalfOpen) {
-    self.write = writeAfterFIN;
-    self.destroySoon();
-  }
-
-  // internal end event so that we know that the actual socket
-  // is no longer readable, and we can start the shutdown
-  // procedure. No need to wait for all the data to be consumed.
-  self.emit('_socketEnd');
 }