-
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
Assertion failed in 12.16.0 #31752
Comments
Bisected to 65e5a8a |
Ugh … I’m pretty sure that’s a V8 bug, and I’ll see if I can find anything on it, but I guess it would be fine to flip the order of the calls around again only for the main instance as a temporary workaround. |
@arcanis It’s hard to actually get access to a failing process, and since you’re most likely more familiar with yarn’s internals than me, I figured I’d ask: As part of the output of your reproduction, each crash backtrace is preceded by information like this:
However, running that command in that directory doesn’t seem to lead to a crash… any suggestions? |
I think I managed to make a basic repro! Clone the repository, then in one tab:
This should give you two things: a failing test directory (similar to the one you noticed), and the url for a mock registry (
This should reproduce the problem. You can run the |
@arcanis I’m afraid that doesn’t quite work for me (x64 Linux, Ubuntu 19.10), partly because this seems flaky and running This is definitely related to WASM usage, which might make it a bit easier to figure out what is causing this. I don’t think fully understanding the issue would be required for a fix, but it would be really nice to have a regression test for this… |
This works around a situation in which the V8 WASM code calls into the platform while the Isolate is being disposed. This goes against the V8 API constract for `v8::Platform`. In lieu of a proper fix, it should be okay to keep the Isolate registered; the race condition fixed by 25447d8 cannot occur for the `NodeMainInstance`’s Isolate, as it is the last one to exit in any given Node.js process. This partially reverts 25447d8. Refs: nodejs#30909 Refs: nodejs#31752
This works around a situation in which the V8 WASM code calls into the platform while the Isolate is being disposed. This goes against the V8 API constract for `v8::Platform`. In lieu of a proper fix, it should be okay to keep the Isolate registered; the race condition fixed by 25447d8 cannot occur for the `NodeMainInstance`’s Isolate, as it is the last one to exit in any given Node.js process. This partially reverts 25447d8. Refs: #30909 Refs: #31752 PR-URL: #31795 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This works around a situation in which the V8 WASM code calls into the platform while the Isolate is being disposed. This goes against the V8 API constract for `v8::Platform`. In lieu of a proper fix, it should be okay to keep the Isolate registered; the race condition fixed by 25447d8 cannot occur for the `NodeMainInstance`’s Isolate, as it is the last one to exit in any given Node.js process. This partially reverts 25447d8. Refs: #30909 Refs: #31752 PR-URL: #31795 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
This works around a situation in which the V8 WASM code calls into the platform while the Isolate is being disposed. This goes against the V8 API constract for `v8::Platform`. In lieu of a proper fix, it should be okay to keep the Isolate registered; the race condition fixed by 25447d8 cannot occur for the `NodeMainInstance`’s Isolate, as it is the last one to exit in any given Node.js process. This partially reverts 25447d8. Refs: #30909 Refs: #31752 PR-URL: #31795 Reviewed-By: Denys Otrishko <shishugi@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
Fwiw, I’ve opened a V8 CL in https://chromium-review.googlesource.com/c/v8/v8/+/2061548 and it currently looks like there’s actually no correct ordering of disposing an Isolate and detaching it from the platform, either one will have a race condition. Let’s see how the conversation there goes. 😬 |
Discard tasks silently that are posted when the Isolate is being disposed. It is not possible to avoid a race condition window between unregistering the Isolate with the platform and disposing it in which background tasks and the Isolate deinit steps themselves may lead to new tasks being posted. The only sensible action in that case is discarding the tasks. Fixes: nodejs#31752 Fixes: https://bugs.chromium.org/p/v8/issues/detail?id=10104 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/2061548 Refs: nodejs#31795 Refs: nodejs#30909
Discard tasks silently that are posted when the Isolate is being disposed. It is not possible to avoid a race condition window between unregistering the Isolate with the platform and disposing it in which background tasks and the Isolate deinit steps themselves may lead to new tasks being posted. The only sensible action in that case is discarding the tasks. Fixes: #31752 Fixes: https://bugs.chromium.org/p/v8/issues/detail?id=10104 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/2061548 Refs: #31795 Refs: #30909 PR-URL: #31853 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Discard tasks silently that are posted when the Isolate is being disposed. It is not possible to avoid a race condition window between unregistering the Isolate with the platform and disposing it in which background tasks and the Isolate deinit steps themselves may lead to new tasks being posted. The only sensible action in that case is discarding the tasks. Fixes: #31752 Fixes: https://bugs.chromium.org/p/v8/issues/detail?id=10104 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/2061548 Refs: #31795 Refs: #30909 PR-URL: #31853 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Discard tasks silently that are posted when the Isolate is being disposed. It is not possible to avoid a race condition window between unregistering the Isolate with the platform and disposing it in which background tasks and the Isolate deinit steps themselves may lead to new tasks being posted. The only sensible action in that case is discarding the tasks. Fixes: #31752 Fixes: https://bugs.chromium.org/p/v8/issues/detail?id=10104 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/2061548 Refs: #31795 Refs: #30909 PR-URL: #31853 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Discard tasks silently that are posted when the Isolate is being disposed. It is not possible to avoid a race condition window between unregistering the Isolate with the platform and disposing it in which background tasks and the Isolate deinit steps themselves may lead to new tasks being posted. The only sensible action in that case is discarding the tasks. Fixes: #31752 Fixes: https://bugs.chromium.org/p/v8/issues/detail?id=10104 Refs: https://chromium-review.googlesource.com/c/v8/v8/+/2061548 Refs: #31795 Refs: #30909 PR-URL: #31853 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
What steps will reproduce the bug?
How often does it reproduce? Is there a required condition?
It seemed to crash consistently across all three systems. The same tests are always failing.
What is the expected behavior?
Tests should pass like here (built yesterday with Node 12.15.0):
https://github.com/yarnpkg/berry/runs/438612900
What do you see instead?
Tests are failing (built just now with Node 12.16.0):
https://github.com/yarnpkg/berry/runs/441525249
The text was updated successfully, but these errors were encountered: