From 4a9b795aae3e301abab78f3e2ab1e37db3015f08 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
---
 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);
     }));
   }));