-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Investigate flaky inspector/test-inspector #8669
Comments
@eugeneo is actively investigating this issue. |
I am currently looking into it. There's a chance that this is a genuine bug (looks like the server does not complete the shutdown once the frontend disconnects). I have tried looping the test on Mac and Ubuntu64 instance but was not able to reproduce. |
By expecting the code, I see one potential race condition (though I do not know how to trigger it) - this commit tries to avoid it: https://github.com/eugeneo/node/commit/2fc65bfdfd703010630879afcc4cc8b22bf28a39 Please try this on CI, if it works I will create a proper PR (will have to remove log statements, fix commit message, etc.) |
@eugeneo I just ran the branch that commit was on through the stress test job on centos5-64 and all but one of the runs still had the same output (one of them was missing the last line -- Stress test results are here (I stopped it once I saw it had failed 80 times). |
@eugeneo Ok, new stress test results containing new output: https://ci.nodejs.org/job/node-stress-single-test/928/nodes=centos5-64/console |
|
Thank you for your help. CentOS5 32-bit failure seems to be gone now (there was a race condition in disconnect during Node shutdown, hopefully that was cause). CentOS5 64 failure - I do not understand it yet, looks like child process stops responding for some reason (or the test fails to interpret the output). I have updated #8672 with different tracing (on the child process side this time) and would like to ask you do another CI run. It still runs on Ubuntu 64 without any problems... |
CentOS5 64 test run: https://ci.nodejs.org/job/node-stress-single-test/929/nodes=centos5-64/console |
Thanks. Test indicates that child process is not "seeing" some messages sent by a test process. I am looking into it, will update the PR once I have a fix ready. |
FWIW, this test is also failing in CI on Raspberry Pi 1 devices:
It's entirely possible that whatever fixes it for CentOS5 might fix it for Raspberry Pi 1 too. If the issue is memory or CPU constraints, we may want to skip the test on Raspberry Pi 1. |
Stress tests uncovered 2 race conditions, when IO events happened during V8 entering event loop on pause or during Node.js shutdown. Fixes: nodejs#8669 PR-URL: nodejs#8672 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Aleksey Kozyatinskiy <[email protected]>
Stress tests uncovered 2 race conditions, when IO events happened during V8 entering event loop on pause or during Node.js shutdown. Fixes: #8669 PR-URL: #8672 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Aleksey Kozyatinskiy <[email protected]>
Stress tests uncovered 2 race conditions, when IO events happened during V8 entering event loop on pause or during Node.js shutdown. Fixes: #8669 PR-URL: #8672 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Aleksey Kozyatinskiy <[email protected]>
https://ci.nodejs.org/job/node-test-commit-linux/5220/nodes=centos5-64/console
Output:
The text was updated successfully, but these errors were encountered: