FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: fix WriteStream autoClose order · nodejs/node@7eed9d6 · GitHub

/ node Public

Commit 7eed9d6

Browse files
ronag
authored and
MylesBorins
committed
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 Backport-PR-URL: #32163 PR-URL: #31790 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent 897b1d2 commit 7eed9d6

4 files changed

Lines changed: 31 additions & 26 deletions

File tree

‎lib/internal/fs/streams.js‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,11 @@ function WriteStream(path, options) {
300300
options.emitClose = false;
301301
}
302302

303+
if (options.autoDestroy === undefined) {
304+
options.autoDestroy = options.autoClose === undefined ?
305+
true : (options.autoClose || false);
306+
}
307+
303308
this[kFs] = options.fs || fs;
304309
if (typeof this[kFs].open !== 'function') {
305310
throw new ERR_INVALID_ARG_TYPE('options.fs.open', 'function',
@@ -343,7 +348,7 @@ function WriteStream(path, options) {
343348
this.mode = options.mode === undefined ? 0o666 : options.mode;
344349

345350
this.start = options.start;
346-
this.autoClose = options.autoClose === undefined ? true : !!options.autoClose;
351+
this.autoClose = options.autoDestroy;
347352
this.pos = undefined;
348353
this.bytesWritten = 0;
349354
this.closed = false;
@@ -371,10 +376,6 @@ WriteStream.prototype._final = function(callback) {
371376
});
372377
}
373378

374-
if (this.autoClose) {
375-
this.destroy();
376-
}
377-
378379
callback();
379380
};
380381

@@ -425,9 +426,6 @@ WriteStream.prototype._write = function(data, encoding, cb) {
425426
}
426427

427428
if (er) {
428-
if (this.autoClose) {
429-
this.destroy();
430-
}
431429
return cb(er);
432430
}
433431
this.bytesWritten += bytes;

‎test/parallel/test-file-write-stream.js‎

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
// USE OR OTHER DEALINGS IN THE SOFTWARE.
2121

2222
'use strict';
23-
require('../common');
23+
const common = require('../common');
2424
const assert = require('assert');
2525

2626
const path = require('path');
@@ -46,9 +46,9 @@ file
4646
callbacks.open++;
4747
assert.strictEqual(typeof fd, 'number');
4848
})
49-
.on('error', function(err) {
50-
throw err;
51-
})
49+
.on('error', common.mustCall((err) => {
50+
assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END');
51+
}))
5252
.on('drain', function() {
5353
console.error('drain!', callbacks.drain);
5454
callbacks.drain++;
@@ -61,21 +61,12 @@ file
6161
}
6262
})
6363
.on('close', function() {
64-
console.error('close!');
6564
assert.strictEqual(file.bytesWritten, EXPECTED.length * 2);
6665

6766
callbacks.close++;
68-
assert.throws(
69-
() => {
70-
console.error('write after end should not be allowed');
71-
file.write('should not work anymore');
72-
},
73-
{
74-
code: 'ERR_STREAM_WRITE_AFTER_END',
75-
name: 'Error',
76-
message: 'write after end'
77-
}
78-
);
67+
file.write('should not work anymore', common.mustCall((err) => {
68+
assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END');
69+
}));
7970

8071
fs.unlinkSync(fn);
8172
});

‎test/parallel/test-fs-read-stream-autoClose.js‎

Lines changed: 16 additions & 0 deletions
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‎

Lines changed: 2 additions & 2 deletions
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)
Back | FazBrowse Home | New Git URL