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.

Sign up for GitHub

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

Jump to bottom

wpt/test-structured-clone is flaky #49852

Closed
joyeecheung opened this issue Sep 25, 2023 · 6 comments · Fixed by #50026
Closed

wpt/test-structured-clone is flaky #49852

joyeecheung opened this issue Sep 25, 2023 · 6 comments · Fixed by #50026
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. windows Issues and PRs related to the Windows platform.

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Sep 25, 2023 •
edited
Loading

Test

wpt/test-structured-clone

Platform

Linux ARM64, Linux x64, Windows

Console output

01:13:46     [UNEXPECTED_FAILURE][FAIL] Blob unpaired high surrogate (invalid utf-8)
01:13:46     promise_test: Unhandled rejection with value: object "Error: Unable to deserialize cloned data."
01:13:46         at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
01:13:46     Command: /home/iojs/build/workspace/node-test-commit-linux/out/Release/node  /home/iojs/build/workspace/node-test-commit-linux/test/wpt/test-structured-clone.js 'structured-clone.any.js'
01:13:46
Reason wpt/test-structured-clone
Type JS_TEST_FAILURE
Failed PR 5 (#49559, #49745, #48434, #49778, #49790)
Appeared test-ibm-alpine315_container-x64-1, test-rackspace-win2019_vs2019-x64-2, test-equinix-ubuntu2004_container-armv7l-1, test-equinix-ubuntu2004_container-armv7l-2, test-digitalocean-ubuntu1804_sharedlibs_container-x64-3
First CI https://ci.nodejs.org/job/node-test-pull-request/54106/
Last CI https://ci.nodejs.org/job/node-test-pull-request/54166/

Additional information

nodejs/reliability#675
@joyeecheung joyeecheung added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Sep 25, 2023
@github-actions github-actions bot added linux Issues and PRs related to the Linux platform. windows Issues and PRs related to the Windows platform. labels Sep 25, 2023
@joyeecheung
Copy link
Member Author

It seems strange that this is a flake, the error should be thrown by V8 when the object cannot be deserialized somehow.

Unfortunately we don't have a lot of information about where the exception comes from, the JS stack is lost due to asynchronicity, maybe we can tweak the wpt harness a bit to capture the stack (not sure if it's already lost by then though)

@joyeecheung
Copy link
Member Author

cc @legendecas as you've recently touched the serdes part

@debadree25
Copy link
Member

debadree25 commented Sep 26, 2023 •
edited
Loading

I wonder if could be related to #49713 compare_Blob function uses Response().arrayBuffer() which internally would be using streams to get the data. will try to reproduce this

@joyeecheung
Copy link
Member Author

That could be a cause, we can try stress-testing a revert

@debadree25
Copy link
Member

Does it not seem to be happening on macos? I tried locally with --repeat 1000 but no reproduction would be weird if this was os specific

@joyeecheung joyeecheung mentioned this issue Sep 27, 2023
Closed
@legendecas
Copy link
Member

Sorry for the delay, I was on a trip last week. I'm going to take a look as soon as possible.

@legendecas legendecas mentioned this issue Oct 3, 2023
Merged
nodejs-github-bot pushed a commit that referenced this issue Oct 10, 2023
@legendecas
src: avoid making JSTransferable wrapper object weak
78a1570 
JSTransferable wrapper object is a short-lived wrapper in the scope of
the serialization or the deserialization. Make the JSTransferable
wrapper object pointer as a strongly-referenced detached BaseObjectPtr
so that a JSTransferable wrapper object and its target object will never
be garbage-collected during a ser-des process, and the wrapper object
will be immediately destroyed when the process is completed.

PR-URL: #50026
Fixes: #49852
Fixes: #49844
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@joyeecheung joyeecheung mentioned this issue Oct 26, 2023
Open
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
@legendecas @alexfernandez
src: avoid making JSTransferable wrapper object weak
9311860 
JSTransferable wrapper object is a short-lived wrapper in the scope of
the serialization or the deserialization. Make the JSTransferable
wrapper object pointer as a strongly-referenced detached BaseObjectPtr
so that a JSTransferable wrapper object and its target object will never
be garbage-collected during a ser-des process, and the wrapper object
will be immediately destroyed when the process is completed.

PR-URL: nodejs#50026
Fixes: nodejs#49852
Fixes: nodejs#49844
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
debadree25 pushed a commit to debadree25/node that referenced this issue Apr 15, 2024
@legendecas @debadree25
src: avoid making JSTransferable wrapper object weak
b27b3a8 
JSTransferable wrapper object is a short-lived wrapper in the scope of
the serialization or the deserialization. Make the JSTransferable
wrapper object pointer as a strongly-referenced detached BaseObjectPtr
so that a JSTransferable wrapper object and its target object will never
be garbage-collected during a ser-des process, and the wrapper object
will be immediately destroyed when the process is completed.

PR-URL: nodejs#50026
Fixes: nodejs#49852
Fixes: nodejs#49844
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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. windows Issues and PRs related to the Windows platform.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

src: avoid making JSTransferable wrapper object weak legendecas/node
3 participants
@joyeecheung @legendecas @debadree25