Skip to content

Commit 90dfd42

Browse files
committed
stream: fix early drain after Utf8Stream reopen
If a 'ready' listener starts a write after reopen(), the reopen path still emits 'drain' from a nextTick before that write completes, so a listener reading the file on 'drain' can observe it empty. This is the race behind the test-fastutf8stream-reopen flake deflaked on the test side in 2e8a4b1. Skip the extra emit when a write is in flight: #release() emits the real 'drain' once the write completes, so no event is lost. The regression test defers the reopened file's fs.write by one setImmediate, deterministically landing the write after the nextTick on which the premature 'drain' used to fire. Refs: 2e8a4b1a8ce Signed-off-by: Matteo Collina <hello@matteocollina.com>
1 parent 2f469df commit 90dfd42

2 files changed

Lines changed: 45 additions & 1 deletion

File tree

lib/internal/streams/fast-utf8-stream.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,9 @@ class Utf8Stream extends EventEmitter {
500500
// start
501501
if ((!this.#writing && this.#len > this.#minLength) || this.#flushPending) {
502502
this.#actualWrite();
503-
} else if (reopening) {
503+
} else if (reopening && !this.#writing) {
504+
// Do not emit 'drain' if a 'ready' listener started a write:
505+
// #release() will emit the real 'drain' when that write completes.
504506
process.nextTick(() => this.emit('drain'));
505507
}
506508
};

test/parallel/test-fastutf8stream-reopen.js

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const {
88
openSync,
99
readFile,
1010
renameSync,
11+
write,
1112
} = require('node:fs');
1213
const { Utf8Stream } = require('node:fs');
1314
const { join } = require('node:path');
@@ -27,6 +28,47 @@ function getTempFile() {
2728
runTests(false);
2829
runTests(true);
2930

31+
// A write started by a 'ready' listener after reopen() must complete before
32+
// 'drain' is emitted. Deferring the reopened file's write by one setImmediate
33+
// makes the write land after the nextTick on which reopen() used to emit a
34+
// premature 'drain'. Async mode only: sync mode writes before 'ready'.
35+
{
36+
const dest = getTempFile();
37+
const after = dest + '-new';
38+
const stream = new Utf8Stream({
39+
dest,
40+
minLength: 0,
41+
sync: false,
42+
fs: {
43+
write(fd, buf, enc, cb) {
44+
if (stream.file === after) {
45+
setImmediate(() => write(fd, buf, enc, cb));
46+
return;
47+
}
48+
return write(fd, buf, enc, cb);
49+
},
50+
},
51+
});
52+
53+
assert.ok(stream.write('hello world\n'));
54+
55+
stream.once('drain', common.mustCall(() => {
56+
stream.reopen(after);
57+
58+
stream.once('ready', common.mustCall(() => {
59+
assert.ok(stream.write('after reopen\n'));
60+
61+
stream.once('drain', common.mustCall(() => {
62+
assert.strictEqual(stream.writing, false);
63+
readFile(after, 'utf8', common.mustSucceed((data) => {
64+
assert.strictEqual(data, 'after reopen\n');
65+
stream.end();
66+
}));
67+
}));
68+
}));
69+
}));
70+
}
71+
3072
function runTests(sync) {
3173

3274
{

0 commit comments

Comments
 (0)