-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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 test-domain-error-types in CI #30498
Comments
This is happening a lot. I'm not sure it's necessary to paste more of these in here at this point, but here's one more.
|
today seen again:
|
Backtrace with optimized V8 debug:
I’ll try to get one from a non-optimized build but this is most likely a V8 GC bug. @nodejs/v8 |
Full stack trace:
|
I’m not really sure how to debug this correctly … valgrind is impractical here, I can’t run the test file in debug mode under it in less than three hours and even then I’d assume the test would still be flaky. If the @nodejs/v8 team has any guidance/ideas for next steps, I’m all ears :) |
This looks a lot like node/deps/v8/src/common/globals.h Line 529 in eb30b2c
|
Do we think we're likely to have something we can do here soon-ish or should we mark the test as flaky (or, probably better, skip on the one platform where there's a problem, if possible)? |
Skip it for now on the one CI configuration where it fails frequently? #30629 |
Until nodejs#30498 is resolved, skip test-domain-error-types on debug builds. PR-URL: nodejs#30629 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
Original commit message: [heap] Ensure SyntheticModule is initialized before next allocation Ensure that all fields of `SyntheticModule` are set before creating the exports hash table for it, because the latter may trigger garbage collection, leading to crashes. This has been causing failures in the Node.js CI over the last weeks, after making the creating of synthetic modules part of Node’s startup sequence. (I am generally not very familiar with this part of the V8 code and there might be a better way, or possibly a way to add a reliable regression test, that I am not aware of.) Refs: nodejs#30498 Refs: nodejs#30648 Change-Id: I32da4b7bd888c6ec1421f34f5bd52e7bad154c1e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1939752 Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#65247} Refs: v8/v8@ca5b0ec Fixes: nodejs#30498 Fixes: nodejs#30648
Ensure that all fields of `SyntheticModule` are set before creating the exports hash table for it, because the latter may trigger garbage collection, leading to crashes. This has been causing failures in the Node.js CI over the last weeks, after making the creating of synthetic modules part of Node’s startup sequence. (I am generally not very familiar with this part of the V8 code and there might be a better way, or possibly a way to add a reliable regression test, that I am not aware of.) Refs: nodejs/node#30498 Refs: nodejs/node#30648 Change-Id: I32da4b7bd888c6ec1421f34f5bd52e7bad154c1e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1939752 Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#65247}
Original commit message: [heap] Ensure SyntheticModule is initialized before next allocation Ensure that all fields of `SyntheticModule` are set before creating the exports hash table for it, because the latter may trigger garbage collection, leading to crashes. This has been causing failures in the Node.js CI over the last weeks, after making the creating of synthetic modules part of Node’s startup sequence. (I am generally not very familiar with this part of the V8 code and there might be a better way, or possibly a way to add a reliable regression test, that I am not aware of.) Refs: #30498 Refs: #30648 Change-Id: I32da4b7bd888c6ec1421f34f5bd52e7bad154c1e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1939752 Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#65247} Refs: https://github.com/v8/v8/commit/ \ ca5b0ec2722d2af4551c01ca78921fa16a26ae72 Fixes: #30498 Fixes: #30648 PR-URL: #30708 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Original commit message: [heap] Ensure SyntheticModule is initialized before next allocation Ensure that all fields of `SyntheticModule` are set before creating the exports hash table for it, because the latter may trigger garbage collection, leading to crashes. This has been causing failures in the Node.js CI over the last weeks, after making the creating of synthetic modules part of Node’s startup sequence. (I am generally not very familiar with this part of the V8 code and there might be a better way, or possibly a way to add a reliable regression test, that I am not aware of.) Refs: #30498 Refs: #30648 Change-Id: I32da4b7bd888c6ec1421f34f5bd52e7bad154c1e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1939752 Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#65247} Refs: https://github.com/v8/v8/commit/ \ ca5b0ec2722d2af4551c01ca78921fa16a26ae72 Fixes: #30498 Fixes: #30648 PR-URL: #30708 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Original commit message: [heap] Ensure SyntheticModule is initialized before next allocation Ensure that all fields of `SyntheticModule` are set before creating the exports hash table for it, because the latter may trigger garbage collection, leading to crashes. This has been causing failures in the Node.js CI over the last weeks, after making the creating of synthetic modules part of Node’s startup sequence. (I am generally not very familiar with this part of the V8 code and there might be a better way, or possibly a way to add a reliable regression test, that I am not aware of.) Refs: #30498 Refs: #30648 Change-Id: I32da4b7bd888c6ec1421f34f5bd52e7bad154c1e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1939752 Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#65247} Refs: https://github.com/v8/v8/commit/ \ ca5b0ec2722d2af4551c01ca78921fa16a26ae72 Fixes: #30498 Fixes: #30648 PR-URL: #30708 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
test/parallel/test-domain-error-types.js
is flaky on containered shared-lib debug builds in CI.Example failures:
https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu1804_sharedlibs_debug_x64/15829/consoleText
https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu1804_sharedlibs_debug_x64/15824/consoleText
https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu1804_sharedlibs_debug_x64/15820/consoleText
Not sure who is in a position to dig in to figure this out so I might be pinging a bit more widely than some people may prefer, so apologies in advance if so. Pings: @nodejs/build @nodejs/testing @nodejs/domains @nodejs/docker @nodejs/diagnostics
The text was updated successfully, but these errors were encountered: