Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flaky test-stream-readable-unpipe-resume #54133

Open
targos opened this issue Jul 31, 2024 · 9 comments
Open

Flaky test-stream-readable-unpipe-resume #54133

targos opened this issue Jul 31, 2024 · 9 comments
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. linux Issues and PRs related to the Linux platform.

Comments

@targos
Copy link
Member

targos commented Jul 31, 2024

Test

test-stream-readable-unpipe-resume

Platform

Linux x64

Console output

14:09:40 not ok 3695 parallel/test-stream-readable-unpipe-resume
14:09:40   ---
14:09:40   duration_ms: 120099.24600
14:09:40   severity: fail
14:09:40   exitcode: -15
14:09:40   stack: |-
14:09:40     timeout
14:09:40   ...

Build links

Additional information

No response

@targos targos added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Jul 31, 2024
@github-actions github-actions bot added the linux Issues and PRs related to the Linux platform. label Jul 31, 2024
@targos
Copy link
Member Author

targos commented Aug 1, 2024

@lpinca
Copy link
Member

lpinca commented Sep 11, 2024

I can reproduce on Ubuntu.

diff --git a/test/parallel/test-stream-readable-unpipe-resume.js b/test/parallel/test-stream-readable-unpipe-resume.js
index b40f724bcc..3eeae52f5f 100644
--- a/test/parallel/test-stream-readable-unpipe-resume.js
+++ b/test/parallel/test-stream-readable-unpipe-resume.js
@@ -13,7 +13,9 @@ const transformStream = new stream.Transform({
   })
 });
 
-readStream.on('end', common.mustCall());
+readStream.on('end', common.mustCall(function () {
+  console.log('end');
+}));
 
 readStream
   .pipe(transformStream)
$ ./tools/test.py --repeat=10000 test/parallel/test-stream-readable-unpipe-resume
=== release test-stream-readable-unpipe-resume ===
Path: parallel/test-stream-readable-unpipe-resume
end
Command: out/Release/node /home/luigi/node/test/parallel/test-stream-readable-unpipe-resume.js
--- TIMEOUT ---


[05:21|% 100|+ 9999|-   1]: Done

Failed tests:
out/Release/node /home/luigi/node/test/parallel/test-stream-readable-unpipe-resume.js

This is yet another case (see #52550 (comment)) where the test correctly finishes (note "end" in the output) but the process does not exit.

@targos
Copy link
Member Author

targos commented Sep 11, 2024

I wish we were able to reproduce these more reliably, because I don't know how to debug something that only happens once or twice every 10k runs!

@lpinca
Copy link
Member

lpinca commented Sep 11, 2024

It might be only a coincidence or a reduced failure rate, but even in this case I get no failures with the --jitless flag.

@RedYetiDev
Copy link
Member

RedYetiDev commented Oct 15, 2024

Hypothetically, as a short-term solution, we could call process.exit() from these tests, but that acts like a bandaid, and not a patch.

- readStream.on('end', common.mustCall());
+ readStream.on('end', common.mustCall(() => process.exit()));

@lpinca
Copy link
Member

lpinca commented Oct 15, 2024

@RedYetiDev it does not help, see #52964 (comment).

@RedYetiDev
Copy link
Member

Insterestingly, it seems to not fail if you change the line to:

readStream.on('end', common.mustCall(() => queueMicrotask(process.exit)));

I ran 10,000 runs with this change: 0 failures
I ran 2,000 runs without this change: 4 failures

Although this could be a coincidence?

@lpinca
Copy link
Member

lpinca commented Oct 17, 2024

Although this could be a coincidence?

Yes, I think so. For example, this

diff --git a/test/parallel/test-net-write-fully-async-hex-string.js b/test/parallel/test-net-write-fully-async-hex-string.js
index 37b5cd75c1..b37f2acefc 100644
--- a/test/parallel/test-net-write-fully-async-hex-string.js
+++ b/test/parallel/test-net-write-fully-async-hex-string.js
@@ -30,3 +30,7 @@ const server = net.createServer(common.mustCall(function(conn) {
     writeLoop();
   }));
 }));
+
+server.on('close', function () {
+  queueMicrotask(process.exit);
+});

makes no difference.

@RedYetiDev
Copy link
Member

This whole issue is strange. I wonder what task is hanging the process 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. linux Issues and PRs related to the Linux platform.
Projects
None yet
Development

No branches or pull requests

3 participants