From e13a37e49d9f9f495cf96c825a64b5754766d348 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Sat, 30 Nov 2019 15:39:44 +0100
Subject: [PATCH] stream: ensure finish is emitted in next tick

When using end() it was possible for 'finish' to
be emitted synchronously.

PR-URL: https://github.com/nodejs/node/pull/30733
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 lib/_stream_writable.js                       | 34 ++++++++++++-------
 .../test-internal-fs-syncwritestream.js       |  4 ++-
 test/parallel/test-stream-writable-destroy.js | 10 +++---
 .../parallel/test-stream-writable-finished.js | 13 +++++++
 4 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js
index 0e0d87ae1d198d..d853441b3cc844 100644
--- a/lib/_stream_writable.js
+++ b/lib/_stream_writable.js
@@ -698,30 +698,40 @@ function prefinish(stream, state) {
   }
 }
 
-function finishMaybe(stream, state) {
+function finishMaybe(stream, state, sync) {
   const need = needFinish(state);
   if (need) {
     prefinish(stream, state);
     if (state.pendingcb === 0) {
-      state.finished = true;
-      stream.emit('finish');
-
-      if (state.autoDestroy) {
-        // In case of duplex streams we need a way to detect
-        // if the readable side is ready for autoDestroy as well
-        const rState = stream._readableState;
-        if (!rState || (rState.autoDestroy && rState.endEmitted)) {
-          stream.destroy();
-        }
+      state.pendingcb++;
+      if (sync) {
+        process.nextTick(finish, stream, state);
+      } else {
+        finish(stream, state);
       }
     }
   }
   return need;
 }
 
+function finish(stream, state) {
+  state.pendingcb--;
+  state.finished = true;
+  stream.emit('finish');
+
+  if (state.autoDestroy) {
+    // In case of duplex streams we need a way to detect
+    // if the readable side is ready for autoDestroy as well
+    const rState = stream._readableState;
+    if (!rState || (rState.autoDestroy && rState.endEmitted)) {
+      stream.destroy();
+    }
+  }
+}
+
 function endWritable(stream, state, cb) {
   state.ending = true;
-  finishMaybe(stream, state);
+  finishMaybe(stream, state, true);
   if (cb) {
     if (state.finished)
       process.nextTick(cb);
diff --git a/test/parallel/test-internal-fs-syncwritestream.js b/test/parallel/test-internal-fs-syncwritestream.js
index 383b70512c7e53..bafa5fd8f4624f 100644
--- a/test/parallel/test-internal-fs-syncwritestream.js
+++ b/test/parallel/test-internal-fs-syncwritestream.js
@@ -70,5 +70,7 @@ const filename = path.join(tmpdir.path, 'sync-write-stream.txt');
   assert.strictEqual(stream.fd, fd);
 
   stream.end();
-  assert.strictEqual(stream.fd, null);
+  stream.on('close', common.mustCall(() => {
+    assert.strictEqual(stream.fd, null);
+  }));
 }
diff --git a/test/parallel/test-stream-writable-destroy.js b/test/parallel/test-stream-writable-destroy.js
index 30e4503c05773a..1186c618634cab 100644
--- a/test/parallel/test-stream-writable-destroy.js
+++ b/test/parallel/test-stream-writable-destroy.js
@@ -238,13 +238,15 @@ const assert = require('assert');
   // called again.
   const write = new Writable({
     write: common.mustNotCall(),
-    final: common.mustCall((cb) => cb(), 2)
+    final: common.mustCall((cb) => cb(), 2),
+    autoDestroy: true
   });
 
   write.end();
-  write.destroy();
-  write._undestroy();
-  write.end();
+  write.once('close', common.mustCall(() => {
+    write._undestroy();
+    write.end();
+  }));
 }
 
 {
diff --git a/test/parallel/test-stream-writable-finished.js b/test/parallel/test-stream-writable-finished.js
index a5dfc060256a02..dfe87a9005db8c 100644
--- a/test/parallel/test-stream-writable-finished.js
+++ b/test/parallel/test-stream-writable-finished.js
@@ -28,3 +28,16 @@ const assert = require('assert');
     assert.strictEqual(writable.writableFinished, true);
   }));
 }
+
+{
+  // Emit finish asynchronously
+
+  const w = new Writable({
+    write(chunk, encoding, cb) {
+      cb();
+    }
+  });
+
+  w.end();
+  w.on('finish', common.mustCall());
+}