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

fix: init v8 platform once on main thread #20495

Merged

Conversation

lucacasonato
Copy link
Member

@lucacasonato lucacasonato commented Sep 14, 2023

This is a mitigation for segfaults happening in V8 on CPU with MPK (memory protected keys).
After much trail and error we found that unless V8 platform is initialized on main thread the
segfaults start appears once JIT kicks in. "deno test" and "deno bench" were impacted by
this problem.

Fixes #19926
Fixes #20243
Fixes #20450

@bartlomieju bartlomieju changed the title wip: init v8 platform once on main thread for deno test fix: init v8 platform once on main thread Sep 14, 2023
@bartlomieju bartlomieju merged commit 06ece56 into denoland:main Sep 14, 2023
@bartlomieju bartlomieju deleted the deno_test_v8_platform_init_once branch September 14, 2023 22:17
mmastrac pushed a commit to denoland/deno_core that referenced this pull request Jan 24, 2024
Due to V8's PKU feature, only the thread that initialized the V8
platform or those spawned from it can access the isolates; other threads
will get a segmentation fault. Since each test runs in its thread, they
crash on newer CPUs that support PKU.

This issue has two possible solutions:
1. Initialize the platform once in the main thread.
2. Turn off the PKU feature during testing.

This issue was already encountered for the `deno test` command, which
was resolved by initializing the platform in the main thread
(denoland/deno#20495).

In the case of rust tests, the same solution cannot be used, as the
default test runner does not allow running code on the main thread
before any test runs.

My proposed solution is to add a feature flag to deno_core, which, when
enabled, uses the unprotected platform. An alternative solution could be
to call `deno_core::JsRuntime::init_platform` at the beginning of each
test.

This PR will also be a step towards fixing denoland/deno#21439
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants