Skip to content

Fix unsoundness in JsValueStore by replacing unsafe raw pointer mutation of Arc with safe OnceLock - #5408

Open
mansiverma897993 wants to merge 2 commits into
boa-dev:mainfrom
mansiverma897993:fix/unsound-jsvaluestore-replace
Open

Fix unsoundness in JsValueStore by replacing unsafe raw pointer mutation of Arc with safe OnceLock#5408
mansiverma897993 wants to merge 2 commits into
boa-dev:mainfrom
mansiverma897993:fix/unsound-jsvaluestore-replace

Conversation

@mansiverma897993

Copy link
Copy Markdown
Contributor

This Pull Request fixes/closes #5392.

It changes the following:

  • Fix unsoundness in JsValueStore by replacing unsafe raw pointer mutation of Arc with safe OnceLock

@mansiverma897993
mansiverma897993 requested a review from a team as a code owner June 20, 2026 08:28
@github-actions github-actions Bot added the Waiting On Review Waiting on reviews from the maintainers label Jun 20, 2026
@github-actions github-actions Bot added this to the v1.0.0 milestone Jun 20, 2026
@github-actions github-actions Bot added C-Tests Issues and PRs related to the tests. C-Runtime Issues and PRs related to Boa's runtime features labels Jun 20, 2026
@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown

Test262 conformance changes

Test result main count PR count difference
Total 53,125 53,125 0
Passed 51,073 51,073 0
Ignored 1,482 1,482 0
Failed 570 570 0
Panics 0 0 0
Conformance 96.14% 96.14% 0.00%

Tested main commit: 93a9e31a83bbaa15bbd8b687e61639ffc53bbef1
Tested PR commit: 357b23d64961869634715e7956f37de50ab45956
Compare commits: 93a9e31...357b23d

@codecov

codecov Bot commented Jun 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.64516% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.81%. Comparing base (6ddc2b4) to head (357b23d).
⚠️ Report is 1019 commits behind head on main.

Files with missing lines Patch % Lines
core/engine/src/module/loader/mod.rs 0.00% 3 Missing ⚠️
core/runtime/src/fetch/fetchers.rs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5408       +/-   ##
===========================================
+ Coverage   47.24%   62.81%   +15.57%     
===========================================
  Files         476      530       +54     
  Lines       46892    59108    +12216     
===========================================
+ Hits        22154    37130    +14976     
+ Misses      24738    21978     -2760     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hansl hansl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the macro change part of this PR? Feels like it should be its own PR, can you split them?

@mansiverma897993
mansiverma897993 force-pushed the fix/unsound-jsvaluestore-replace branch from 15efccc to 09cb647 Compare August 24, 2026 04:59
@mansiverma897993

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @hansl.
I've split the PR as requested the macro change is no longer part of this PR. I removed the fix: allow deriving both TryFromJs and TryIntoJs with custom converters commit (that change was already merged separately in #5403), leaving only the JsValueStore OnceLock fix.
I also rebased onto the latest main, which resolves the conflicts from the store module move (core/runtime/src/store → core/wintertc/src/store).
All checks pass locally: boa_wintertc and boa_runtime tests, clippy with -D warnings, and cargo fmt --check.

Fix the unused-async-trait-impl and useless-format clippy lints
introduced by the Rust 1.98 stable toolchain in existing ModuleLoader
and Fetcher implementations.
@github-actions github-actions Bot added the C-VM Issues and PRs related to the Boa Virtual Machine. label Aug 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Runtime Issues and PRs related to Boa's runtime features C-Tests Issues and PRs related to the tests. C-VM Issues and PRs related to the Boa Virtual Machine. Waiting On Review Waiting on reviews from the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dubious use of unsafe in runtime::store::JsValueStore.replace()

2 participants