net: fix ambiguity in EOF handling

`end` MUST always be emitted **before** `close`. However, if a handle
will invoke `uv_close_cb` immediately, or in the same JS tick - `close`
may be emitted first.

PR-URL: #9066
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Ilkka Myller <[email protected]>

Author: indutny

lib/net.js

@@ -571,14 +571,16 @@ function onread(nread, buffer) {
 
   debug('EOF');
 
+  // push a null to signal the end of data.
+  // Do it before `maybeDestroy` for correct order of events:
+  // `end` -> `close`
+  self.push(null);
+
   if (self._readableState.length === 0) {
     self.readable = false;
     maybeDestroy(self);
   }
 
-  // push a null to signal the end of data.
-  self.push(null);
-
   // 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.

test/parallel/test-net-end-close.js

@@ -0,0 +1,26 @@
+'use strict';
+require('../common');
+const assert = require('assert');
+const net = require('net');
+
+const uv = process.binding('uv');
+
+const s = new net.Socket({
+  handle: {
+    readStart: function() {
+      process.nextTick(() => this.onread(uv.UV_EOF, null));
+    },
+    close: (cb) => process.nextTick(cb)
+  },
+  writable: false
+});
+s.resume();
+
+const events = [];
+
+s.on('end', () => events.push('end'));
+s.on('close', () => events.push('close'));
+
+process.on('exit', () => {
+  assert.deepStrictEqual(events, [ 'end', 'close' ]);
+});