fix(shadowrealm): unpin ShadowRealm.prototype at host teardown - #1209
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Limit details: You’ve used all 5 included reviews currently available. Your 53 included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThe ShadowRealm host destructor now unpins ChangesShadowRealm lifecycle
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change releases the ShadowRealm prototype pin during host teardown to prevent the reported multi-file warm-up crash while preserving realm isolation; no actionable merge-blocking risk remains beyond normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Comment |
Web Tooling Benchmark
18 pinned Web Tooling workloads; 18 workloads produced at least one Goccia sample. Raw results from 1 sample per workload; full stdout/stderr for failures and min/max/CV stay in the |
JetStream 3 Performance Barometer
Geomean reference ratio: QuickJS 27.03×; Node.js 300.55×. 1.00× means aligned; values above 1.00× mean Goccia was proportionally slower after normalizing JetStream’s higher-is-better score. This is a directional barometer across runtimes with different goals, not a product ranking. Raw samples and failure details remain in the |
Suite TimingTest Runner (interpreted: 12,763 passed; bytecode: 12,763 passed)
MemoryGC rows aggregate the main thread plus all worker thread-local GCs. Test runner worker shutdown frees thread-local heaps in bulk; that shutdown reclamation is not counted as GC collections or collected objects.
Benchmarks (interpreted: 440; bytecode: 440)
MemoryGC rows aggregate the main thread plus all worker thread-local GCs. Benchmark runner performs explicit between-file collections, so collection and collected-object counts can be much higher than the test runner.
Boot
Empty-script ( Measured on ubuntu-latest x64. |
Benchmark Results440 benchmarks · PR vs same-runner Interpreted: 🟢 34 improved · 🔴 25 regressed · 381 unchanged · avg +1.2% Typical per-run noise (median variance): interpreted ±2.5%, bytecode ±1.5%. Deltas within noise overlap and read as unchanged. arraybuffer.js — Interp: 14 unch. · avg -0.4% · Bytecode: 🔴 2, 12 unch. · avg -1.4%
arrays.js — Interp: 🟢 1, 🔴 1, 17 unch. · avg +0.4% · Bytecode: 🟢 1, 18 unch. · avg +1.1%
async-await.js — Interp: 6 unch. · avg +2.1% · Bytecode: 6 unch. · avg -2.6%
async-generators.js — Interp: 2 unch. · avg +0.1% · Bytecode: 2 unch. · avg -5.8%
atomics.js — Interp: 6 unch. · avg -0.5% · Bytecode: 🟢 1, 5 unch. · avg +0.7%
base64.js — Interp: 🟢 2, 8 unch. · avg -0.6% · Bytecode: 🟢 1, 🔴 2, 7 unch. · avg -0.9%
classes.js — Interp: 🔴 1, 30 unch. · avg +0.1% · Bytecode: 🟢 1, 🔴 5, 25 unch. · avg -1.0%
closures.js — Interp: 🟢 1, 10 unch. · avg +1.3% · Bytecode: 11 unch. · avg +0.6%
collections.js — Interp: 🟢 1, 11 unch. · avg +2.0% · Bytecode: 🔴 1, 11 unch. · avg +0.7%
csv.js — Interp: 13 unch. · avg -0.6% · Bytecode: 🟢 1, 12 unch. · avg -0.9%
destructuring.js — Interp: 22 unch. · avg -0.8% · Bytecode: 🔴 1, 21 unch. · avg -0.2%
fibonacci.js — Interp: 🔴 1, 7 unch. · avg +2.3% · Bytecode: 8 unch. · avg -0.4%
float16array.js — Interp: 🔴 3, 29 unch. · avg +0.1% · Bytecode: 🟢 1, 🔴 1, 30 unch. · avg +0.4%
for-in/for-in.js — Interp: 🔴 1, 2 unch. · avg -0.4% · Bytecode: 3 unch. · avg -1.8%
for-of.js — Interp: 🟢 3, 4 unch. · avg +7.9% · Bytecode: 🟢 1, 🔴 3, 3 unch. · avg -3.2%
generators.js — Interp: 🔴 1, 3 unch. · avg -4.2% · Bytecode: 🟢 2, 2 unch. · avg +0.4%
intl.js — Interp: 6 unch. · avg +1.1% · Bytecode: 🔴 1, 5 unch. · avg -2.2%
iterators.js — Interp: 🔴 6, 36 unch. · avg -1.8% · Bytecode: 🟢 2, 🔴 7, 33 unch. · avg -0.5%
json.js — Interp: 🟢 6, 17 unch. · avg +3.0% · Bytecode: 🔴 1, 22 unch. · avg +0.0%
jsx.jsx — Interp: 🟢 2, 19 unch. · avg +2.8% · Bytecode: 🔴 1, 20 unch. · avg +0.0%
modules.js — Interp: 9 unch. · avg +2.1% · Bytecode: 9 unch. · avg -2.3%
numbers.js — Interp: 12 unch. · avg +3.1% · Bytecode: 12 unch. · avg +1.1%
objects.js — Interp: 🟢 1, 7 unch. · avg +5.1% · Bytecode: 8 unch. · avg +0.3%
promises.js — Interp: 🟢 2, 10 unch. · avg +5.6% · Bytecode: 🟢 1, 🔴 1, 10 unch. · avg -0.9%
property-access.js — Interp: 5 unch. · avg -0.6% · Bytecode: 5 unch. · avg +8.6%
regexp.js — Interp: 🟢 1, 12 unch. · avg +1.4% · Bytecode: 🟢 1, 🔴 2, 10 unch. · avg +1.4%
strings.js — Interp: 🟢 2, 17 unch. · avg +0.4% · Bytecode: 🟢 2, 🔴 1, 16 unch. · avg +1.0%
temporal.js — Interp: 🟢 1, 5 unch. · avg +1.2% · Bytecode: 6 unch. · avg +0.3%
tsv.js — Interp: 🔴 4, 5 unch. · avg -7.2% · Bytecode: 🟢 1, 🔴 2, 6 unch. · avg +1.8%
typed-arrays.js — Interp: 🟢 7, 15 unch. · avg +21.2% · Bytecode: 🟢 7, 🔴 4, 11 unch. · avg +17.4%
uint8array-encoding.js — Interp: 🟢 4, 🔴 2, 12 unch. · avg -3.2% · Bytecode: 🟢 2, 🔴 10, 6 unch. · avg -7.5%
weak-collections.js — Interp: 🔴 5, 10 unch. · avg -8.6% · Bytecode: 🟢 1, 🔴 6, 8 unch. · avg -13.1%
Deterministic profile diffDeterministic profile diff: no significant changes. Measured on ubuntu-latest x64. Each PR run also builds the |
test262 Conformance
Areas closest to 100%
Per-test deltas (+0 / -0 / timeout +2 / -2)New timeouts (2):
Resolved timeouts (2):
Steady-state failures and timeouts are non-blocking; PASS → non-timeout failure transitions fail the conformance gate. Measured on ubuntu-latest x64, bytecode mode. Areas grouped by the first two test262 path components; minimum 25 attempted tests, areas already at 100% excluded. Δ vs main compares against the most recent cached |
AWFY Results
Geomean Ratios
14 pinned AWFY benchmarks. Medians from 5 interleaved samples per engine; raw JSON includes min/max/CV and is attached as the |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. Your current included review allowance is based on your included PR review attempts over the past 7 days. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 12 minutes. |
5ab8635 to
78b6808
Compare
Release the GC pin on the child realm's prototype in the ShadowRealm host destructor so a later per-file collection cannot mark a freed realm graph. Fixes the multi-file unsafe-shadowrealm runtime-module warm-up segfault.
78b6808 to
4a7e703
Compare
Summary
node:async_hooks,goccia:csv,goccia:timers) from a directory withunsafe-shadowrealm: truein a multi-file run crashed during warm-up.ShadowRealm.prototypefor the engine's lifetime but never released it at teardown, so the next per-fileGC.Collectmarked a surviving-but-dangling prototype and walked into a freed realm graph (EXC_BAD_ACCESSinMarkRoots). The host destructor now unpins the prototype before freeing child realms; a pure lifetime fix that grants the child no capability and preserves realm isolation.Testing
unsafe-shadowrealmwarm-up regression test that faults before the fix./format.pas --checkand doc checks clean