test: deflake fastutf8stream destroy and reopen tests by christianaurichzm · Pull Request #65554 · nodejs/node

christianaurichzm · 2026-08-26T03:45:17Z

test-fastutf8stream-destroy and test-fastutf8stream-reopen can
read their destination files before the underlying write operation has
completed.

In test-fastutf8stream-destroy, readFile() is called immediately
after destroy(), while an asynchronous fs.write() may still be in
flight.

In test-fastutf8stream-reopen, the reads are ordered on 'drain'.
However, 'drain' indicates that the internal buffer has drained
sufficiently to allow continued writing; it does not guarantee that the
write being asserted has completed. In the reopen path, a 'drain' is
scheduled with process.nextTick() after 'ready', so it can be observed
before the subsequent write has completed.

Order these reads on the 'write' event instead, which is emitted from
#release() after the underlying write operation completes.

For the synchronous reopen path, the 'write' listener is registered
before calling write(), since the event can be emitted from within the
write() call.

This only changes test synchronization. No Utf8Stream runtime behavior
is changed.

Testing

Before the change:

test-fastutf8stream-destroy: 34 failures / 2880 runs
test-fastutf8stream-reopen: 54 failures / 2880 runs

After the change, locally on Linux x64:

test-fastutf8stream-destroy: 0 failures / 3600 runs
test-fastutf8stream-reopen: 0 failures / 3600 runs
tools/test.py -J --repeat=40: 80/80
eslint: passes
core-validate-commit: passes

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md
Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>

christianaurichzm · 2026-08-26T03:57:10Z

cc @mcollina, since you were involved in the earlier fastutf8stream flake investigation in #59638.

This is test-only and ready for CI. Could you add the request-ci label when convenient? Thanks!

codecov · 2026-08-26T06:25:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (7b6b21a) to head (db8a012).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65554 +/- ##
==========================================
+ Coverage 90.05% 90.06% +0.01% 
==========================================
Files 751 751 Lines 254420 254420 Branches 47975 47972 -3 ==========================================
+ Hits 229121 229156 +35 + Misses 16483 16445 -38 - Partials 8816 8819 +3

see 32 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-26T09:16:47Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76512/

github-actions · 2026-08-26T10:42:06Z

Fast-track has been requested by @panva. Please 👍 to approve.

panva · 2026-08-26T11:56:56Z

pending node-stress-single-test started by @sxa

Edit: appears to ✅

sxa · 2026-08-26T14:05:45Z

Edit: appears to ✅
The one you linked to (848) was a different test and yours hasn't run through to completion yet yet - the ones I have are:

844 - Current main - passed 100 iterations without failures which is not helpful :-)
845 - Same as 844 but with fs: use sized reads for large files in readFileUtf8 #65328 reverted in case that was the cause
851 - also seemed ok on the rhel8/arm64 machine - also ran through very quickly so I've started another 1000 iteration run at https://ci.nodejs.org/job/node-stress-single-test/853/

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

christianaurichzm · 2026-08-26T15:58:53Z

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

You're right that those runs don't prove much yet. One important difference from the reproducer I used is concurrency.

The race is between the fs.write() that is still in flight and the subsequent readFile() path, which is not ordered after that write has completed. I was able to make it reproduce often enough by running each test file in 24 concurrent processes, with a separate TEST_THREAD_ID per process so they do not share a tmpdir.

Across two Linux x64 environments, the rates varied quite a bit for destroy and were more stable for reopen:

test-fastutf8stream-destroy: 25 / 8640 (0.29%) and 34 / 2880 (1.18%)
test-fastutf8stream-reopen: 152 / 8640 (1.76%) and 54 / 2880 (1.88%)

Taking the lower destroy rate and assuming roughly independent runs, 100 runs still have about a 75% chance of producing zero failures, and even 1000 runs have about a 5% chance. So 844 coming back clean is an expected outcome rather than evidence against the race, and 853 may well come back clean too without disproving anything. Sequential runs on an otherwise idle worker are also much less favorable for reproducing the contention I was using.

Every failure I observed had the same signature: readFile() returning '' at the assertion this patch reorders. I did not observe another failure mode in the stress runs.

With the patch applied, both tests produced 0 failures in 8640 runs each in the environment that produced the 0.29% and 1.76% baselines, where those rates would correspond to roughly 25 and 152 failures.

The ordering issue itself does not depend on reproducing the flake every time: drain is not a completion barrier for the write being asserted, whereas the write event used here is emitted from #release() after the underlying write operation completes.

Happy to share the stress setup or run other configurations if useful.

nodejs-github-bot · 2026-08-26T18:01:48Z

Landed in 2e8a4b1

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com> PR-URL: #65554 Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>

christianaurichzm · 2026-08-26T03:45:17Z

test-fastutf8stream-destroy and test-fastutf8stream-reopen can
read their destination files before the underlying write operation has
completed.

In test-fastutf8stream-destroy, readFile() is called immediately
after destroy(), while an asynchronous fs.write() may still be in
flight.

In test-fastutf8stream-reopen, the reads are ordered on 'drain'.
However, 'drain' indicates that the internal buffer has drained
sufficiently to allow continued writing; it does not guarantee that the
write being asserted has completed. In the reopen path, a 'drain' is
scheduled with process.nextTick() after 'ready', so it can be observed
before the subsequent write has completed.

Order these reads on the 'write' event instead, which is emitted from
#release() after the underlying write operation completes.

For the synchronous reopen path, the 'write' listener is registered
before calling write(), since the event can be emitted from within the
write() call.

This only changes test synchronization. No Utf8Stream runtime behavior
is changed.

Testing

Before the change:

test-fastutf8stream-destroy: 34 failures / 2880 runs
test-fastutf8stream-reopen: 54 failures / 2880 runs

After the change, locally on Linux x64:

test-fastutf8stream-destroy: 0 failures / 3600 runs
test-fastutf8stream-reopen: 0 failures / 3600 runs
tools/test.py -J --repeat=40: 80/80
eslint: passes
core-validate-commit: passes

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md
Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>

christianaurichzm · 2026-08-26T03:57:10Z

cc @mcollina, since you were involved in the earlier fastutf8stream flake investigation in #59638.

This is test-only and ready for CI. Could you add the request-ci label when convenient? Thanks!

codecov · 2026-08-26T06:25:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (7b6b21a) to head (db8a012).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65554 +/- ##
==========================================
+ Coverage 90.05% 90.06% +0.01% 
==========================================
Files 751 751 Lines 254420 254420 Branches 47975 47972 -3 ==========================================
+ Hits 229121 229156 +35 + Misses 16483 16445 -38 - Partials 8816 8819 +3

see 32 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-26T09:16:47Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76512/

github-actions · 2026-08-26T10:42:06Z

Fast-track has been requested by @panva. Please 👍 to approve.

panva · 2026-08-26T11:56:56Z

pending node-stress-single-test started by @sxa

Edit: appears to ✅

sxa · 2026-08-26T14:05:45Z

Edit: appears to ✅
The one you linked to (848) was a different test and yours hasn't run through to completion yet yet - the ones I have are:

844 - Current main - passed 100 iterations without failures which is not helpful :-)
845 - Same as 844 but with fs: use sized reads for large files in readFileUtf8 #65328 reverted in case that was the cause
851 - also seemed ok on the rhel8/arm64 machine - also ran through very quickly so I've started another 1000 iteration run at https://ci.nodejs.org/job/node-stress-single-test/853/

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

christianaurichzm · 2026-08-26T15:58:53Z

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

You're right that those runs don't prove much yet. One important difference from the reproducer I used is concurrency.

The race is between the fs.write() that is still in flight and the subsequent readFile() path, which is not ordered after that write has completed. I was able to make it reproduce often enough by running each test file in 24 concurrent processes, with a separate TEST_THREAD_ID per process so they do not share a tmpdir.

Across two Linux x64 environments, the rates varied quite a bit for destroy and were more stable for reopen:

test-fastutf8stream-destroy: 25 / 8640 (0.29%) and 34 / 2880 (1.18%)
test-fastutf8stream-reopen: 152 / 8640 (1.76%) and 54 / 2880 (1.88%)

Taking the lower destroy rate and assuming roughly independent runs, 100 runs still have about a 75% chance of producing zero failures, and even 1000 runs have about a 5% chance. So 844 coming back clean is an expected outcome rather than evidence against the race, and 853 may well come back clean too without disproving anything. Sequential runs on an otherwise idle worker are also much less favorable for reproducing the contention I was using.

Every failure I observed had the same signature: readFile() returning '' at the assertion this patch reorders. I did not observe another failure mode in the stress runs.

With the patch applied, both tests produced 0 failures in 8640 runs each in the environment that produced the 0.29% and 1.76% baselines, where those rates would correspond to roughly 25 and 152 failures.

The ordering issue itself does not depend on reproducing the flake every time: drain is not a completion barrier for the write being asserted, whereas the write event used here is emitted from #release() after the underlying write operation completes.

Happy to share the stress setup or run other configurations if useful.

nodejs-github-bot · 2026-08-26T18:01:48Z

Landed in 2e8a4b1

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com> PR-URL: #65554 Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>

christianaurichzm · 2026-08-26T03:45:17Z

test-fastutf8stream-destroy and test-fastutf8stream-reopen can
read their destination files before the underlying write operation has
completed.

In test-fastutf8stream-destroy, readFile() is called immediately
after destroy(), while an asynchronous fs.write() may still be in
flight.

In test-fastutf8stream-reopen, the reads are ordered on 'drain'.
However, 'drain' indicates that the internal buffer has drained
sufficiently to allow continued writing; it does not guarantee that the
write being asserted has completed. In the reopen path, a 'drain' is
scheduled with process.nextTick() after 'ready', so it can be observed
before the subsequent write has completed.

Order these reads on the 'write' event instead, which is emitted from
#release() after the underlying write operation completes.

For the synchronous reopen path, the 'write' listener is registered
before calling write(), since the event can be emitted from within the
write() call.

This only changes test synchronization. No Utf8Stream runtime behavior
is changed.

Testing

Before the change:

test-fastutf8stream-destroy: 34 failures / 2880 runs
test-fastutf8stream-reopen: 54 failures / 2880 runs

After the change, locally on Linux x64:

test-fastutf8stream-destroy: 0 failures / 3600 runs
test-fastutf8stream-reopen: 0 failures / 3600 runs
tools/test.py -J --repeat=40: 80/80
eslint: passes
core-validate-commit: passes

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md
Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>

christianaurichzm · 2026-08-26T03:57:10Z

cc @mcollina, since you were involved in the earlier fastutf8stream flake investigation in #59638.

This is test-only and ready for CI. Could you add the request-ci label when convenient? Thanks!

codecov · 2026-08-26T06:25:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (7b6b21a) to head (db8a012).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65554 +/- ##
==========================================
+ Coverage 90.05% 90.06% +0.01% 
==========================================
Files 751 751 Lines 254420 254420 Branches 47975 47972 -3 ==========================================
+ Hits 229121 229156 +35 + Misses 16483 16445 -38 - Partials 8816 8819 +3

see 32 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-26T09:16:47Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76512/

github-actions · 2026-08-26T10:42:06Z

Fast-track has been requested by @panva. Please 👍 to approve.

panva · 2026-08-26T11:56:56Z

pending node-stress-single-test started by @sxa

Edit: appears to ✅

sxa · 2026-08-26T14:05:45Z

Edit: appears to ✅
The one you linked to (848) was a different test and yours hasn't run through to completion yet yet - the ones I have are:

844 - Current main - passed 100 iterations without failures which is not helpful :-)
845 - Same as 844 but with fs: use sized reads for large files in readFileUtf8 #65328 reverted in case that was the cause
851 - also seemed ok on the rhel8/arm64 machine - also ran through very quickly so I've started another 1000 iteration run at https://ci.nodejs.org/job/node-stress-single-test/853/

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

christianaurichzm · 2026-08-26T15:58:53Z

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

You're right that those runs don't prove much yet. One important difference from the reproducer I used is concurrency.

The race is between the fs.write() that is still in flight and the subsequent readFile() path, which is not ordered after that write has completed. I was able to make it reproduce often enough by running each test file in 24 concurrent processes, with a separate TEST_THREAD_ID per process so they do not share a tmpdir.

Across two Linux x64 environments, the rates varied quite a bit for destroy and were more stable for reopen:

test-fastutf8stream-destroy: 25 / 8640 (0.29%) and 34 / 2880 (1.18%)
test-fastutf8stream-reopen: 152 / 8640 (1.76%) and 54 / 2880 (1.88%)

Taking the lower destroy rate and assuming roughly independent runs, 100 runs still have about a 75% chance of producing zero failures, and even 1000 runs have about a 5% chance. So 844 coming back clean is an expected outcome rather than evidence against the race, and 853 may well come back clean too without disproving anything. Sequential runs on an otherwise idle worker are also much less favorable for reproducing the contention I was using.

Every failure I observed had the same signature: readFile() returning '' at the assertion this patch reorders. I did not observe another failure mode in the stress runs.

With the patch applied, both tests produced 0 failures in 8640 runs each in the environment that produced the 0.29% and 1.76% baselines, where those rates would correspond to roughly 25 and 152 failures.

The ordering issue itself does not depend on reproducing the flake every time: drain is not a completion barrier for the write being asserted, whereas the write event used here is emitted from #release() after the underlying write operation completes.

Happy to share the stress setup or run other configurations if useful.

nodejs-github-bot · 2026-08-26T18:01:48Z

Landed in 2e8a4b1

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com> PR-URL: #65554 Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>

christianaurichzm · 2026-08-26T03:45:17Z

test-fastutf8stream-destroy and test-fastutf8stream-reopen can
read their destination files before the underlying write operation has
completed.

In test-fastutf8stream-destroy, readFile() is called immediately
after destroy(), while an asynchronous fs.write() may still be in
flight.

In test-fastutf8stream-reopen, the reads are ordered on 'drain'.
However, 'drain' indicates that the internal buffer has drained
sufficiently to allow continued writing; it does not guarantee that the
write being asserted has completed. In the reopen path, a 'drain' is
scheduled with process.nextTick() after 'ready', so it can be observed
before the subsequent write has completed.

Order these reads on the 'write' event instead, which is emitted from
#release() after the underlying write operation completes.

For the synchronous reopen path, the 'write' listener is registered
before calling write(), since the event can be emitted from within the
write() call.

This only changes test synchronization. No Utf8Stream runtime behavior
is changed.

Testing

Before the change:

test-fastutf8stream-destroy: 34 failures / 2880 runs
test-fastutf8stream-reopen: 54 failures / 2880 runs

After the change, locally on Linux x64:

test-fastutf8stream-destroy: 0 failures / 3600 runs
test-fastutf8stream-reopen: 0 failures / 3600 runs
tools/test.py -J --repeat=40: 80/80
eslint: passes
core-validate-commit: passes

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md
Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>

christianaurichzm · 2026-08-26T03:57:10Z

cc @mcollina, since you were involved in the earlier fastutf8stream flake investigation in #59638.

This is test-only and ready for CI. Could you add the request-ci label when convenient? Thanks!

codecov · 2026-08-26T06:25:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (7b6b21a) to head (db8a012).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65554 +/- ##
==========================================
+ Coverage 90.05% 90.06% +0.01% 
==========================================
Files 751 751 Lines 254420 254420 Branches 47975 47972 -3 ==========================================
+ Hits 229121 229156 +35 + Misses 16483 16445 -38 - Partials 8816 8819 +3

see 32 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-26T09:16:47Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76512/

github-actions · 2026-08-26T10:42:06Z

Fast-track has been requested by @panva. Please 👍 to approve.

panva · 2026-08-26T11:56:56Z

pending node-stress-single-test started by @sxa

Edit: appears to ✅

sxa · 2026-08-26T14:05:45Z

Edit: appears to ✅
The one you linked to (848) was a different test and yours hasn't run through to completion yet yet - the ones I have are:

844 - Current main - passed 100 iterations without failures which is not helpful :-)
845 - Same as 844 but with fs: use sized reads for large files in readFileUtf8 #65328 reverted in case that was the cause
851 - also seemed ok on the rhel8/arm64 machine - also ran through very quickly so I've started another 1000 iteration run at https://ci.nodejs.org/job/node-stress-single-test/853/

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

christianaurichzm · 2026-08-26T15:58:53Z

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

You're right that those runs don't prove much yet. One important difference from the reproducer I used is concurrency.

The race is between the fs.write() that is still in flight and the subsequent readFile() path, which is not ordered after that write has completed. I was able to make it reproduce often enough by running each test file in 24 concurrent processes, with a separate TEST_THREAD_ID per process so they do not share a tmpdir.

Across two Linux x64 environments, the rates varied quite a bit for destroy and were more stable for reopen:

test-fastutf8stream-destroy: 25 / 8640 (0.29%) and 34 / 2880 (1.18%)
test-fastutf8stream-reopen: 152 / 8640 (1.76%) and 54 / 2880 (1.88%)

Taking the lower destroy rate and assuming roughly independent runs, 100 runs still have about a 75% chance of producing zero failures, and even 1000 runs have about a 5% chance. So 844 coming back clean is an expected outcome rather than evidence against the race, and 853 may well come back clean too without disproving anything. Sequential runs on an otherwise idle worker are also much less favorable for reproducing the contention I was using.

Every failure I observed had the same signature: readFile() returning '' at the assertion this patch reorders. I did not observe another failure mode in the stress runs.

With the patch applied, both tests produced 0 failures in 8640 runs each in the environment that produced the 0.29% and 1.76% baselines, where those rates would correspond to roughly 25 and 152 failures.

The ordering issue itself does not depend on reproducing the flake every time: drain is not a completion barrier for the write being asserted, whereas the write event used here is emitted from #release() after the underlying write operation completes.

Happy to share the stress setup or run other configurations if useful.

nodejs-github-bot · 2026-08-26T18:01:48Z

Landed in 2e8a4b1

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com> PR-URL: #65554 Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>

christianaurichzm · 2026-08-26T03:45:17Z

test-fastutf8stream-destroy and test-fastutf8stream-reopen can
read their destination files before the underlying write operation has
completed.

In test-fastutf8stream-destroy, readFile() is called immediately
after destroy(), while an asynchronous fs.write() may still be in
flight.

In test-fastutf8stream-reopen, the reads are ordered on 'drain'.
However, 'drain' indicates that the internal buffer has drained
sufficiently to allow continued writing; it does not guarantee that the
write being asserted has completed. In the reopen path, a 'drain' is
scheduled with process.nextTick() after 'ready', so it can be observed
before the subsequent write has completed.

Order these reads on the 'write' event instead, which is emitted from
#release() after the underlying write operation completes.

For the synchronous reopen path, the 'write' listener is registered
before calling write(), since the event can be emitted from within the
write() call.

This only changes test synchronization. No Utf8Stream runtime behavior
is changed.

Testing

Before the change:

test-fastutf8stream-destroy: 34 failures / 2880 runs
test-fastutf8stream-reopen: 54 failures / 2880 runs

After the change, locally on Linux x64:

test-fastutf8stream-destroy: 0 failures / 3600 runs
test-fastutf8stream-reopen: 0 failures / 3600 runs
tools/test.py -J --repeat=40: 80/80
eslint: passes
core-validate-commit: passes

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md
Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>

christianaurichzm · 2026-08-26T03:57:10Z

cc @mcollina, since you were involved in the earlier fastutf8stream flake investigation in #59638.

This is test-only and ready for CI. Could you add the request-ci label when convenient? Thanks!

codecov · 2026-08-26T06:25:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (7b6b21a) to head (db8a012).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65554 +/- ##
==========================================
+ Coverage 90.05% 90.06% +0.01% 
==========================================
Files 751 751 Lines 254420 254420 Branches 47975 47972 -3 ==========================================
+ Hits 229121 229156 +35 + Misses 16483 16445 -38 - Partials 8816 8819 +3

see 32 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-26T09:16:47Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76512/

github-actions · 2026-08-26T10:42:06Z

Fast-track has been requested by @panva. Please 👍 to approve.

panva · 2026-08-26T11:56:56Z

pending node-stress-single-test started by @sxa

Edit: appears to ✅

sxa · 2026-08-26T14:05:45Z

Edit: appears to ✅
The one you linked to (848) was a different test and yours hasn't run through to completion yet yet - the ones I have are:

844 - Current main - passed 100 iterations without failures which is not helpful :-)
845 - Same as 844 but with fs: use sized reads for large files in readFileUtf8 #65328 reverted in case that was the cause
851 - also seemed ok on the rhel8/arm64 machine - also ran through very quickly so I've started another 1000 iteration run at https://ci.nodejs.org/job/node-stress-single-test/853/

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

christianaurichzm · 2026-08-26T15:58:53Z

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

You're right that those runs don't prove much yet. One important difference from the reproducer I used is concurrency.

The race is between the fs.write() that is still in flight and the subsequent readFile() path, which is not ordered after that write has completed. I was able to make it reproduce often enough by running each test file in 24 concurrent processes, with a separate TEST_THREAD_ID per process so they do not share a tmpdir.

Across two Linux x64 environments, the rates varied quite a bit for destroy and were more stable for reopen:

test-fastutf8stream-destroy: 25 / 8640 (0.29%) and 34 / 2880 (1.18%)
test-fastutf8stream-reopen: 152 / 8640 (1.76%) and 54 / 2880 (1.88%)

Taking the lower destroy rate and assuming roughly independent runs, 100 runs still have about a 75% chance of producing zero failures, and even 1000 runs have about a 5% chance. So 844 coming back clean is an expected outcome rather than evidence against the race, and 853 may well come back clean too without disproving anything. Sequential runs on an otherwise idle worker are also much less favorable for reproducing the contention I was using.

Every failure I observed had the same signature: readFile() returning '' at the assertion this patch reorders. I did not observe another failure mode in the stress runs.

With the patch applied, both tests produced 0 failures in 8640 runs each in the environment that produced the 0.29% and 1.76% baselines, where those rates would correspond to roughly 25 and 152 failures.

The ordering issue itself does not depend on reproducing the flake every time: drain is not a completion barrier for the write being asserted, whereas the write event used here is emitted from #release() after the underlying write operation completes.

Happy to share the stress setup or run other configurations if useful.

nodejs-github-bot · 2026-08-26T18:01:48Z

Landed in 2e8a4b1

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com> PR-URL: #65554 Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>

christianaurichzm · 2026-08-26T03:45:17Z

test-fastutf8stream-destroy and test-fastutf8stream-reopen can
read their destination files before the underlying write operation has
completed.

In test-fastutf8stream-destroy, readFile() is called immediately
after destroy(), while an asynchronous fs.write() may still be in
flight.

In test-fastutf8stream-reopen, the reads are ordered on 'drain'.
However, 'drain' indicates that the internal buffer has drained
sufficiently to allow continued writing; it does not guarantee that the
write being asserted has completed. In the reopen path, a 'drain' is
scheduled with process.nextTick() after 'ready', so it can be observed
before the subsequent write has completed.

Order these reads on the 'write' event instead, which is emitted from
#release() after the underlying write operation completes.

For the synchronous reopen path, the 'write' listener is registered
before calling write(), since the event can be emitted from within the
write() call.

This only changes test synchronization. No Utf8Stream runtime behavior
is changed.

Testing

Before the change:

test-fastutf8stream-destroy: 34 failures / 2880 runs
test-fastutf8stream-reopen: 54 failures / 2880 runs

After the change, locally on Linux x64:

test-fastutf8stream-destroy: 0 failures / 3600 runs
test-fastutf8stream-reopen: 0 failures / 3600 runs
tools/test.py -J --repeat=40: 80/80
eslint: passes
core-validate-commit: passes

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md
Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>

christianaurichzm · 2026-08-26T03:57:10Z

cc @mcollina, since you were involved in the earlier fastutf8stream flake investigation in #59638.

This is test-only and ready for CI. Could you add the request-ci label when convenient? Thanks!

codecov · 2026-08-26T06:25:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (7b6b21a) to head (db8a012).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65554 +/- ##
==========================================
+ Coverage 90.05% 90.06% +0.01% 
==========================================
Files 751 751 Lines 254420 254420 Branches 47975 47972 -3 ==========================================
+ Hits 229121 229156 +35 + Misses 16483 16445 -38 - Partials 8816 8819 +3

see 32 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-26T09:16:47Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76512/

github-actions · 2026-08-26T10:42:06Z

Fast-track has been requested by @panva. Please 👍 to approve.

panva · 2026-08-26T11:56:56Z

pending node-stress-single-test started by @sxa

Edit: appears to ✅

sxa · 2026-08-26T14:05:45Z

Edit: appears to ✅
The one you linked to (848) was a different test and yours hasn't run through to completion yet yet - the ones I have are:

844 - Current main - passed 100 iterations without failures which is not helpful :-)
845 - Same as 844 but with fs: use sized reads for large files in readFileUtf8 #65328 reverted in case that was the cause
851 - also seemed ok on the rhel8/arm64 machine - also ran through very quickly so I've started another 1000 iteration run at https://ci.nodejs.org/job/node-stress-single-test/853/

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

christianaurichzm · 2026-08-26T15:58:53Z

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

You're right that those runs don't prove much yet. One important difference from the reproducer I used is concurrency.

The race is between the fs.write() that is still in flight and the subsequent readFile() path, which is not ordered after that write has completed. I was able to make it reproduce often enough by running each test file in 24 concurrent processes, with a separate TEST_THREAD_ID per process so they do not share a tmpdir.

Across two Linux x64 environments, the rates varied quite a bit for destroy and were more stable for reopen:

test-fastutf8stream-destroy: 25 / 8640 (0.29%) and 34 / 2880 (1.18%)
test-fastutf8stream-reopen: 152 / 8640 (1.76%) and 54 / 2880 (1.88%)

Taking the lower destroy rate and assuming roughly independent runs, 100 runs still have about a 75% chance of producing zero failures, and even 1000 runs have about a 5% chance. So 844 coming back clean is an expected outcome rather than evidence against the race, and 853 may well come back clean too without disproving anything. Sequential runs on an otherwise idle worker are also much less favorable for reproducing the contention I was using.

Every failure I observed had the same signature: readFile() returning '' at the assertion this patch reorders. I did not observe another failure mode in the stress runs.

With the patch applied, both tests produced 0 failures in 8640 runs each in the environment that produced the 0.29% and 1.76% baselines, where those rates would correspond to roughly 25 and 152 failures.

The ordering issue itself does not depend on reproducing the flake every time: drain is not a completion barrier for the write being asserted, whereas the write event used here is emitted from #release() after the underlying write operation completes.

Happy to share the stress setup or run other configurations if useful.

nodejs-github-bot · 2026-08-26T18:01:48Z

Landed in 2e8a4b1

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com> PR-URL: #65554 Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>

christianaurichzm · 2026-08-26T03:45:17Z

test-fastutf8stream-destroy and test-fastutf8stream-reopen can
read their destination files before the underlying write operation has
completed.

In test-fastutf8stream-destroy, readFile() is called immediately
after destroy(), while an asynchronous fs.write() may still be in
flight.

In test-fastutf8stream-reopen, the reads are ordered on 'drain'.
However, 'drain' indicates that the internal buffer has drained
sufficiently to allow continued writing; it does not guarantee that the
write being asserted has completed. In the reopen path, a 'drain' is
scheduled with process.nextTick() after 'ready', so it can be observed
before the subsequent write has completed.

Order these reads on the 'write' event instead, which is emitted from
#release() after the underlying write operation completes.

For the synchronous reopen path, the 'write' listener is registered
before calling write(), since the event can be emitted from within the
write() call.

This only changes test synchronization. No Utf8Stream runtime behavior
is changed.

Testing

Before the change:

test-fastutf8stream-destroy: 34 failures / 2880 runs
test-fastutf8stream-reopen: 54 failures / 2880 runs

After the change, locally on Linux x64:

test-fastutf8stream-destroy: 0 failures / 3600 runs
test-fastutf8stream-reopen: 0 failures / 3600 runs
tools/test.py -J --repeat=40: 80/80
eslint: passes
core-validate-commit: passes

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md
Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>

christianaurichzm · 2026-08-26T03:57:10Z

cc @mcollina, since you were involved in the earlier fastutf8stream flake investigation in #59638.

This is test-only and ready for CI. Could you add the request-ci label when convenient? Thanks!

codecov · 2026-08-26T06:25:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (7b6b21a) to head (db8a012).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65554 +/- ##
==========================================
+ Coverage 90.05% 90.06% +0.01% 
==========================================
Files 751 751 Lines 254420 254420 Branches 47975 47972 -3 ==========================================
+ Hits 229121 229156 +35 + Misses 16483 16445 -38 - Partials 8816 8819 +3

see 32 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-26T09:16:47Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76512/

github-actions · 2026-08-26T10:42:06Z

Fast-track has been requested by @panva. Please 👍 to approve.

panva · 2026-08-26T11:56:56Z

pending node-stress-single-test started by @sxa

Edit: appears to ✅

sxa · 2026-08-26T14:05:45Z

Edit: appears to ✅
The one you linked to (848) was a different test and yours hasn't run through to completion yet yet - the ones I have are:

844 - Current main - passed 100 iterations without failures which is not helpful :-)
845 - Same as 844 but with fs: use sized reads for large files in readFileUtf8 #65328 reverted in case that was the cause
851 - also seemed ok on the rhel8/arm64 machine - also ran through very quickly so I've started another 1000 iteration run at https://ci.nodejs.org/job/node-stress-single-test/853/

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

christianaurichzm · 2026-08-26T15:58:53Z

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

You're right that those runs don't prove much yet. One important difference from the reproducer I used is concurrency.

The race is between the fs.write() that is still in flight and the subsequent readFile() path, which is not ordered after that write has completed. I was able to make it reproduce often enough by running each test file in 24 concurrent processes, with a separate TEST_THREAD_ID per process so they do not share a tmpdir.

Across two Linux x64 environments, the rates varied quite a bit for destroy and were more stable for reopen:

test-fastutf8stream-destroy: 25 / 8640 (0.29%) and 34 / 2880 (1.18%)
test-fastutf8stream-reopen: 152 / 8640 (1.76%) and 54 / 2880 (1.88%)

Taking the lower destroy rate and assuming roughly independent runs, 100 runs still have about a 75% chance of producing zero failures, and even 1000 runs have about a 5% chance. So 844 coming back clean is an expected outcome rather than evidence against the race, and 853 may well come back clean too without disproving anything. Sequential runs on an otherwise idle worker are also much less favorable for reproducing the contention I was using.

Every failure I observed had the same signature: readFile() returning '' at the assertion this patch reorders. I did not observe another failure mode in the stress runs.

With the patch applied, both tests produced 0 failures in 8640 runs each in the environment that produced the 0.29% and 1.76% baselines, where those rates would correspond to roughly 25 and 152 failures.

The ordering issue itself does not depend on reproducing the flake every time: drain is not a completion barrier for the write being asserted, whereas the write event used here is emitted from #release() after the underlying write operation completes.

Happy to share the stress setup or run other configurations if useful.

nodejs-github-bot · 2026-08-26T18:01:48Z

Landed in 2e8a4b1

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com> PR-URL: #65554 Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>

christianaurichzm · 2026-08-26T03:45:17Z

test-fastutf8stream-destroy and test-fastutf8stream-reopen can
read their destination files before the underlying write operation has
completed.

In test-fastutf8stream-destroy, readFile() is called immediately
after destroy(), while an asynchronous fs.write() may still be in
flight.

In test-fastutf8stream-reopen, the reads are ordered on 'drain'.
However, 'drain' indicates that the internal buffer has drained
sufficiently to allow continued writing; it does not guarantee that the
write being asserted has completed. In the reopen path, a 'drain' is
scheduled with process.nextTick() after 'ready', so it can be observed
before the subsequent write has completed.

Order these reads on the 'write' event instead, which is emitted from
#release() after the underlying write operation completes.

For the synchronous reopen path, the 'write' listener is registered
before calling write(), since the event can be emitted from within the
write() call.

This only changes test synchronization. No Utf8Stream runtime behavior
is changed.

Testing

Before the change:

test-fastutf8stream-destroy: 34 failures / 2880 runs
test-fastutf8stream-reopen: 54 failures / 2880 runs

After the change, locally on Linux x64:

test-fastutf8stream-destroy: 0 failures / 3600 runs
test-fastutf8stream-reopen: 0 failures / 3600 runs
tools/test.py -J --repeat=40: 80/80
eslint: passes
core-validate-commit: passes

Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md
Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com>

christianaurichzm · 2026-08-26T03:57:10Z

cc @mcollina, since you were involved in the earlier fastutf8stream flake investigation in #59638.

This is test-only and ready for CI. Could you add the request-ci label when convenient? Thanks!

codecov · 2026-08-26T06:25:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.06%. Comparing base (7b6b21a) to head (db8a012).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files

@@ Coverage Diff @@## main #65554 +/- ##
==========================================
+ Coverage 90.05% 90.06% +0.01% 
==========================================
Files 751 751 Lines 254420 254420 Branches 47975 47972 -3 ==========================================
+ Hits 229121 229156 +35 + Misses 16483 16445 -38 - Partials 8816 8819 +3

see 32 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

nodejs-github-bot · 2026-08-26T09:16:47Z

CI: https://ci.nodejs.org/job/node-test-pull-request/76512/

github-actions · 2026-08-26T10:42:06Z

Fast-track has been requested by @panva. Please 👍 to approve.

panva · 2026-08-26T11:56:56Z

pending node-stress-single-test started by @sxa

Edit: appears to ✅

sxa · 2026-08-26T14:05:45Z

Edit: appears to ✅
The one you linked to (848) was a different test and yours hasn't run through to completion yet yet - the ones I have are:

844 - Current main - passed 100 iterations without failures which is not helpful :-)
845 - Same as 844 but with fs: use sized reads for large files in readFileUtf8 #65328 reverted in case that was the cause
851 - also seemed ok on the rhel8/arm64 machine - also ran through very quickly so I've started another 1000 iteration run at https://ci.nodejs.org/job/node-stress-single-test/853/

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

christianaurichzm · 2026-08-26T15:58:53Z

So I'm not sure we've necessarily proven anything as non of those reproduced the original error :-) Have just kicked off another from the current main on four platforms at https://ci.nodejs.org/view/Stress/job/node-stress-single-test/852

You're right that those runs don't prove much yet. One important difference from the reproducer I used is concurrency.

The race is between the fs.write() that is still in flight and the subsequent readFile() path, which is not ordered after that write has completed. I was able to make it reproduce often enough by running each test file in 24 concurrent processes, with a separate TEST_THREAD_ID per process so they do not share a tmpdir.

Across two Linux x64 environments, the rates varied quite a bit for destroy and were more stable for reopen:

test-fastutf8stream-destroy: 25 / 8640 (0.29%) and 34 / 2880 (1.18%)
test-fastutf8stream-reopen: 152 / 8640 (1.76%) and 54 / 2880 (1.88%)

Taking the lower destroy rate and assuming roughly independent runs, 100 runs still have about a 75% chance of producing zero failures, and even 1000 runs have about a 5% chance. So 844 coming back clean is an expected outcome rather than evidence against the race, and 853 may well come back clean too without disproving anything. Sequential runs on an otherwise idle worker are also much less favorable for reproducing the contention I was using.

Every failure I observed had the same signature: readFile() returning '' at the assertion this patch reorders. I did not observe another failure mode in the stress runs.

With the patch applied, both tests produced 0 failures in 8640 runs each in the environment that produced the 0.29% and 1.76% baselines, where those rates would correspond to roughly 25 and 152 failures.

The ordering issue itself does not depend on reproducing the flake every time: drain is not a completion barrier for the write being asserted, whereas the write event used here is emitted from #release() after the underlying write operation completes.

Happy to share the stress setup or run other configurations if useful.

nodejs-github-bot · 2026-08-26T18:01:48Z

Landed in 2e8a4b1

Both tests read the destination file with no ordering guarantee against the fs.write() that Utf8Stream still has in flight, so under load the read can observe an empty file. In test-fastutf8stream-destroy the read is issued right after destroy(). In test-fastutf8stream-reopen it is ordered on 'drain', documented as emitted when the buffer has drained enough to allow continued writing, which says nothing about the bytes being observable in the file. The reopen path also emits a 'drain' of its own from a nextTick before the write has landed. Order both reads on 'write' instead, documented as emitted when a write operation has completed and emitted from #release() once the underlying write returned. In sync mode it is emitted from within write(), so the listener is attached before the write call. No data is lost by Utf8Stream here: re-reading the file after a failed assertion shows the expected content. This corrects an expectation of the tests, not the runtime. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com> PR-URL: #65554 Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-20.md Refs: https://github.com/nodejs/reliability/blob/main/reports/2026-08-26.md Reviewed-By: Shelley Vohr <shelley.vohr@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to Node.js core tests and test infrastructure. labels Aug 26, 2026

MikeMcC399 mentioned this pull request Aug 26, 2026
permission: support URL and Uint8Array as has()/drop() reference #65492
Merged

panva added the flaky-test Issues and PRs involving tests that fail intermittently in CI. label Aug 26, 2026

panva added the review wanted PRs that need review. label Aug 26, 2026

codebytere approved these changes Aug 26, 2026
View reviewed changes

panva added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Aug 26, 2026

panva mentioned this pull request Aug 26, 2026
test: mark platform-specific tests as flaky #65562
Merged

panva added the fast-track PRs proposed for a shorter-than-standard waiting period before landing. label Aug 26, 2026

panva approved these changes Aug 26, 2026
View reviewed changes

panva added the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 26, 2026

nodejs-github-bot merged commit 2e8a4b1 into nodejs:mainAug 26, 2026
102 of 103 checks passed

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Aug 26, 2026

sxa mentioned this pull request Aug 27, 2026
Built-In Node has low disk space nodejs/build#4419
Open