From cc55747f632c54918bca88a070e8f8c7659e0978 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Fri, 14 Feb 2020 13:32:45 +0100
Subject: [PATCH] fs: fix WriteStream autoClose order

WriteStream autoClose was implemented by manually
calling .destroy() instead of using autoDestroy
and callback. This caused some invariants related
to order of events to be broken.

Fixes: https://github.com/nodejs/node/issues/31776

PR-URL: https://github.com/nodejs/node/pull/31790
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
---
 lib/internal/fs/streams.js                       | 12 +++---------
 test/parallel/test-fs-read-stream-autoClose.js   | 16 ++++++++++++++++
 .../test-fs-write-stream-autoclose-option.js     |  4 ++--
 3 files changed, 21 insertions(+), 11 deletions(-)
 create mode 100644 test/parallel/test-fs-read-stream-autoClose.js

diff --git a/lib/internal/fs/streams.js b/lib/internal/fs/streams.js
index c121273861da79..5b37dca74bed63 100644
--- a/lib/internal/fs/streams.js
+++ b/lib/internal/fs/streams.js
@@ -291,7 +291,8 @@ function WriteStream(path, options) {
   options.decodeStrings = true;
 
   if (options.autoDestroy === undefined) {
-    options.autoDestroy = false;
+    options.autoDestroy = options.autoClose === undefined ?
+      true : (options.autoClose || false);
   }
 
   this[kFs] = options.fs || fs;
@@ -337,7 +338,7 @@ function WriteStream(path, options) {
   this.mode = options.mode === undefined ? 0o666 : options.mode;
 
   this.start = options.start;
-  this.autoClose = options.autoClose === undefined ? true : !!options.autoClose;
+  this.autoClose = options.autoDestroy;
   this.pos = undefined;
   this.bytesWritten = 0;
   this.closed = false;
@@ -365,10 +366,6 @@ WriteStream.prototype._final = function(callback) {
     });
   }
 
-  if (this.autoClose) {
-    this.destroy();
-  }
-
   callback();
 };
 
@@ -419,9 +416,6 @@ WriteStream.prototype._write = function(data, encoding, cb) {
     }
 
     if (er) {
-      if (this.autoClose) {
-        this.destroy();
-      }
       return cb(er);
     }
     this.bytesWritten += bytes;
diff --git a/test/parallel/test-fs-read-stream-autoClose.js b/test/parallel/test-fs-read-stream-autoClose.js
new file mode 100644
index 00000000000000..e7989ee7911264
--- /dev/null
+++ b/test/parallel/test-fs-read-stream-autoClose.js
@@ -0,0 +1,16 @@
+'use strict';
+
+const common = require('../common');
+const fs = require('fs');
+const path = require('path');
+const assert = require('assert');
+const tmpdir = require('../common/tmpdir');
+const writeFile = path.join(tmpdir.path, 'write-autoClose.txt');
+tmpdir.refresh();
+
+const file = fs.createWriteStream(writeFile, { autoClose: true });
+
+file.on('finish', common.mustCall(() => {
+  assert.strictEqual(file.destroyed, false);
+}));
+file.end('asd');
diff --git a/test/parallel/test-fs-write-stream-autoclose-option.js b/test/parallel/test-fs-write-stream-autoclose-option.js
index e39f4d615ab7e0..8d205fbc69fb39 100644
--- a/test/parallel/test-fs-write-stream-autoclose-option.js
+++ b/test/parallel/test-fs-write-stream-autoclose-option.js
@@ -27,8 +27,8 @@ function next() {
   stream.end();
   stream.on('finish', common.mustCall(function() {
     assert.strictEqual(stream.closed, false);
-    assert.strictEqual(stream.fd, null);
     stream.on('close', common.mustCall(function() {
+      assert.strictEqual(stream.fd, null);
       assert.strictEqual(stream.closed, true);
       process.nextTick(next2);
     }));
@@ -51,8 +51,8 @@ function next3() {
   stream.end();
   stream.on('finish', common.mustCall(function() {
     assert.strictEqual(stream.closed, false);
-    assert.strictEqual(stream.fd, null);
     stream.on('close', common.mustCall(function() {
+      assert.strictEqual(stream.fd, null);
       assert.strictEqual(stream.closed, true);
     }));
   }));