Skip to content

Commit

Permalink
fs: fix WriteStream autoClose order
Browse files Browse the repository at this point in the history
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: nodejs#31776
  • Loading branch information
ronag committed Feb 14, 2020
1 parent 0875837 commit 4a9b795
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 11 deletions.
12 changes: 3 additions & 9 deletions lib/internal/fs/streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -365,10 +366,6 @@ WriteStream.prototype._final = function(callback) {
});
}

if (this.autoClose) {
this.destroy();
}

callback();
};

Expand Down Expand Up @@ -419,9 +416,6 @@ WriteStream.prototype._write = function(data, encoding, cb) {
}

if (er) {
if (this.autoClose) {
this.destroy();
}
return cb(er);
}
this.bytesWritten += bytes;
Expand Down
16 changes: 16 additions & 0 deletions test/parallel/test-fs-read-stream-autoClose.js
Original file line number Diff line number Diff line change
@@ -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');
4 changes: 2 additions & 2 deletions test/parallel/test-fs-write-stream-autoclose-option.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}));
Expand All @@ -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);
}));
}));
Expand Down

0 comments on commit 4a9b795

Please sign in to comment.