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 sequential/test-http2-settings-flood #31107

Closed
addaleax opened this issue Dec 27, 2019 · 15 comments
Closed

Flaky test sequential/test-http2-settings-flood #31107

addaleax opened this issue Dec 27, 2019 · 15 comments
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. http2 Issues or PRs related to the http2 subsystem. v8 engine Issues and PRs related to the V8 dependency.

Comments

@addaleax
Copy link
Member

  • Version: master
  • Platform: AIX p158a05 1 7 00CCB1C24C00
  • Subsystem: v8?

The test in sequential/test-http2-settings-flood sometimes fails on AIX PPC64 with SIGILL and no other output (about 4/100 times). Bisecting points to @ronag’s cd6b00d, which obviously shouldn’t be causing that issue. It doesn’t reproduce when running the test with --jitless, so my best guess would be that it’s a V8 issue on PPC.

I can’t really debug this any further, the issue doesn’t seem to reproduce under gdb and core dumps don’t seem helpful, so it would be great if somebody in @nodejs/platform-aix (@miladfarca?) could take a look. In the short term, I guess we could also revert the commit above, although that’s obviously not ideal.

@addaleax addaleax added aix Issues and PRs related to the AIX platform. flaky-test Issues and PRs related to the tests with unstable failures on the CI. labels Dec 27, 2019
@miladfarca
Copy link
Contributor

miladfarca commented Jan 6, 2020

I have tested the above on AIX 6.1 as well as pLinux on a loop (~400 times) and it always passes.
Could you please add my key to the above machine:

ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDQZihjEXcY52UZo09CEb29HwOWwXcmwbwZFR4rsACQQyGUexL4fkVrFfwuG8eV1vg5KECsO8LiGY/MHkNIpABaJJoip0Qxgv0pAFtAukIDjLXXOV/VNJjfIto16vOAehRZkmI+BtQP8TjoT2CSyJgvVQcay8BhH52in1LQQsyCi2crHLYzDrrCgY/rAmuVb1MzMnT8mFOdJ8E5RBhjnmc1K4YBKmNTf6yefgbOJssI0lLzp7Q2uytzp3pipg7AO/VqmRn8953UTJS/cOQeBi3nCYGpz4I7kOKHgwbdW1IP/XFfm0KO5daulHQeToRGIE85ntxF314wsYE3ZyeJwKgH

Edit:
Seems like #31146 is causing it

@sam-github
Copy link
Contributor

Gave access, permission already granted in nodejs/build#1706

@addaleax
Copy link
Member Author

addaleax commented Jan 7, 2020

Seems like #31146 is causing it

@miladfarca Can you figure out why it is crashing?

@miladfarca
Copy link
Contributor

@addaleax Yes I will be looking into it, might be related to V8 deoptimization

@miladfarca
Copy link
Contributor

This issue doesn't seem to be specific to PPC or AIX. It is also failing on x64-freebsd on the CI: https://ci.nodejs.org/job/node-test-commit-freebsd/nodes=freebsd11-x64/30698/console

I have done a debug compile on both pLinux and x64 Linux and this test periodically fails on the same DCHEK:

#
# Fatal error in ../deps/v8/src/deoptimizer/deoptimizer.cc, line 253
# Debug check failed: code == topmost_ implies safe_to_deopt_.
#
#
#
#FailureMessage Object: 0x7ffc4970cda0

There seems to be an issue with V8 de-optimizer.

@ronag
Copy link
Member

ronag commented Jan 7, 2020

@miladfarca: good job! would you mind creating a V8 ticket?

@miladfarca
Copy link
Contributor

miladfarca commented Jan 7, 2020

@ronag I've created this ticket but not sure if we need to reproduce it under V8 (and not nodejs), will wait for their reply:
https://bugs.chromium.org/p/v8/issues/detail?id=10101

@Trott
Copy link
Member

Trott commented Jan 8, 2020

@miladfarca Same results with a debug build on MacOS Mojave:

$ tools/test.py --mode=debug --repeat 1000 test/sequential/test-http2-ping-flood.js
=== debug test-http2-ping-flood ===                   
Path: sequential/test-http2-ping-flood
#
# Fatal error in ../deps/v8/src/deoptimizer/deoptimizer.cc, line 253
# Debug check failed: code == topmost_ implies safe_to_deopt_.
#
#
#

@Trott Trott changed the title Flaky test sequential/test-http2-settings-flood on AIX Flaky test sequential/test-http2-settings-flood Jan 8, 2020
@miladfarca
Copy link
Contributor

Thanks @Trott , just to confirm your mac is x64 correct? I'll add to the V8 ticket.

@Trott
Copy link
Member

Trott commented Jan 10, 2020

Thanks @Trott , just to confirm your mac is x64 correct? I'll add to the V8 ticket.

Yes, x64.

@ronag
Copy link
Member

ronag commented Jan 11, 2020

ping @nodejs/v8

@vtjnash
Copy link
Contributor

vtjnash commented Jan 13, 2020

It appears this is already known to be platform agnostic, but just for additional info, it also has been observed on debian9-docker-armv7 (https://ci.nodejs.org/job/node-test-commit-arm/28686/)

@Trott Trott added v8 engine Issues and PRs related to the V8 dependency. http2 Issues or PRs related to the http2 subsystem. and removed aix Issues and PRs related to the AIX platform. labels Jan 26, 2020
@mscdex
Copy link
Contributor

mscdex commented Feb 3, 2020

FWIW this is also occurring in Ubuntu 18.04 containers.

@miladfarca
Copy link
Contributor

@addaleax V8 ticket (https://bugs.chromium.org/p/v8/issues/detail?id=10101) is now closed with a comment suggesting an API misuse in node. Issue might be related to this commit/line which might need to be refactored: 7e6104f#diff-9e96b9359319a317a5a1ee45f11eea67R126

@addaleax
Copy link
Member Author

addaleax commented Feb 5, 2020

@miladfarca Thanks for the info, I’ll try to come up with a fix. They are right here, but I am surprised that ArrayBuffer::Detach() is unsafe for calling here.

Issue might be related to this commit/line which might need to be refactored: 7e6104f#diff-9e96b9359319a317a5a1ee45f11eea67R126

I’d like to avoid that, and instead try to not perform the invalid V8 call here.

addaleax added a commit to addaleax/node that referenced this issue Feb 5, 2020
It looks like it’s virtually impossible at this point to
create “fake” backing stores for objects that don’t fully
own their memory allocations, like the sub-field `js_fields_`
of `Http2Session`. In particular, it turns out that an
`ArrayBuffer` cannot always be easily separated from its
backing store in that situation through by detaching it.

This commit gives the JS-exposed parts of the class its own
memory allocation and its own backing store, simplifying the
code a bit and fixing flakiness coming from it, at the cost
of one additional layer of indirection when accessing the data.

Refs: nodejs#30782
Fixes: nodejs#31107
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. http2 Issues or PRs related to the http2 subsystem. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants