From 9a192a9683851fa43153f8646a90213e5f14fe3c Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 12 Oct 2016 20:39:37 +0000 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/9066 Reviewed-By: Matteo Collina Reviewed-By: Rich Trott Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Ilkka Myller --- lib/net.js | 8 +++++--- test/parallel/test-net-end-close.js | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-net-end-close.js diff --git a/lib/net.js b/lib/net.js index a5b52353b59627..e166fadaa8a075 100644 --- a/lib/net.js +++ b/lib/net.js @@ -560,14 +560,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. diff --git a/test/parallel/test-net-end-close.js b/test/parallel/test-net-end-close.js new file mode 100644 index 00000000000000..d9f33fd7d8d1cf --- /dev/null +++ b/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' ]); +});