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

PR-URL: nodejs#31790
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
ronag authored and sbaayel committed Oct 10, 2023
1 parent 97fea3d commit cc55747
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 cc55747

Please sign in to comment.