Skip to content

Commit cc55747

Browse files
ronagsbaayel
authored andcommitted
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: #31776 PR-URL: #31790 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
1 parent 97fea3d commit cc55747

File tree

3 files changed

+21
-11
lines changed

3 files changed

+21
-11
lines changed

lib/internal/fs/streams.js

+3-9
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,8 @@ function WriteStream(path, options) {
291291
options.decodeStrings = true;
292292

293293
if (options.autoDestroy === undefined) {
294-
options.autoDestroy = false;
294+
options.autoDestroy = options.autoClose === undefined ?
295+
true : (options.autoClose || false);
295296
}
296297

297298
this[kFs] = options.fs || fs;
@@ -337,7 +338,7 @@ function WriteStream(path, options) {
337338
this.mode = options.mode === undefined ? 0o666 : options.mode;
338339

339340
this.start = options.start;
340-
this.autoClose = options.autoClose === undefined ? true : !!options.autoClose;
341+
this.autoClose = options.autoDestroy;
341342
this.pos = undefined;
342343
this.bytesWritten = 0;
343344
this.closed = false;
@@ -365,10 +366,6 @@ WriteStream.prototype._final = function(callback) {
365366
});
366367
}
367368

368-
if (this.autoClose) {
369-
this.destroy();
370-
}
371-
372369
callback();
373370
};
374371

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

421418
if (er) {
422-
if (this.autoClose) {
423-
this.destroy();
424-
}
425419
return cb(er);
426420
}
427421
this.bytesWritten += bytes;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const fs = require('fs');
5+
const path = require('path');
6+
const assert = require('assert');
7+
const tmpdir = require('../common/tmpdir');
8+
const writeFile = path.join(tmpdir.path, 'write-autoClose.txt');
9+
tmpdir.refresh();
10+
11+
const file = fs.createWriteStream(writeFile, { autoClose: true });
12+
13+
file.on('finish', common.mustCall(() => {
14+
assert.strictEqual(file.destroyed, false);
15+
}));
16+
file.end('asd');

test/parallel/test-fs-write-stream-autoclose-option.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ function next() {
2727
stream.end();
2828
stream.on('finish', common.mustCall(function() {
2929
assert.strictEqual(stream.closed, false);
30-
assert.strictEqual(stream.fd, null);
3130
stream.on('close', common.mustCall(function() {
31+
assert.strictEqual(stream.fd, null);
3232
assert.strictEqual(stream.closed, true);
3333
process.nextTick(next2);
3434
}));
@@ -51,8 +51,8 @@ function next3() {
5151
stream.end();
5252
stream.on('finish', common.mustCall(function() {
5353
assert.strictEqual(stream.closed, false);
54-
assert.strictEqual(stream.fd, null);
5554
stream.on('close', common.mustCall(function() {
55+
assert.strictEqual(stream.fd, null);
5656
assert.strictEqual(stream.closed, true);
5757
}));
5858
}));

0 commit comments

Comments
 (0)