fix(runtime): cache Web Crypto method closures instead of reallocating per read

[proggeramlug]

Summary

Reading a Web Crypto method off globalThis.crypto (or crypto.subtle) allocated a fresh closure on every access, so the method had no stable identity: crypto.randomUUID === crypto.randomUUID was false, and each read allocated a new ClosureHeader.

Root cause

globalThis.crypto is backed by a NATIVE_MODULE_CLASS_ID namespace object (crypto.webcrypto), so crypto.randomUUID resolves through vt_get_own_field → webcrypto_method_value (crates/perry-runtime/src/object/global_this/ctor_thunks.rs). That function (and its crypto.subtle counterpart, subtle_crypto_method_value, for the KEM methods encapsulateBits/decapsulateBits/encapsulateKey/decapsulateKey) called plain js_closure_alloc(func_ptr, 0) on every call — a brand-new closure each time a property was read, not just each time the method was called.

This is the same closure-identity contract PR #10630 (Fixes #10554) traced back to js_closure_alloc_singleton: every user-function value materializes into a heap closure, and the singleton allocator (crates/perry-runtime/src/closure/alloc.rs) caches that materialization by func_ptr, giving the same ClosureHeader back on every call for a given code pointer. webcrypto_method_value/subtle_crypto_method_value's thunks take no captures, so the func_ptr already is the method's identity — using the singleton cache instead of a fresh allocation was a direct drop-in.

Fix

crates/perry-runtime/src/object/global_this/ctor_thunks.rs: webcrypto_method_value and subtle_crypto_method_value now call js_closure_alloc_singleton(func_ptr) instead of js_closure_alloc(func_ptr, 0). Two lines changed, plus explanatory comments.

Which globalThis.crypto members were checked

• randomUUID, getRandomValues — both routed through the same broken webcrypto_method_value; both fixed by this change.
• crypto.subtle (the namespace object itself) — already stable: "crypto.subtle" is in should_cache_native_module_namespace's allow-list, so js_create_native_module_namespace returns the same cached object on every read. Unaffected, verified as a control.
• crypto.subtle.encapsulateBits/decapsulateBits/encapsulateKey/decapsulateKey — routed through subtle_crypto_method_value, the same defect. Fixed by the same change.
• crypto.subtle.digest/encrypt/decrypt/sign/verify/deriveBits/deriveKey/exportKey/generateKey/importKey/unwrapKey/wrapKey — these resolve through is_native_module_callable_export → bound_native_callable_export_value, a different, already-correctly-cached mechanism (NATIVE_CALLABLE_EXPORTS, keyed by module\0property). Already stable; verified as controls, unaffected either way.
• node:crypto's randomUUID/randomBytes (default and named imports) — also resolve through bound_native_callable_export_value. Already stable (this is what the issue's own repro showed as already-true); verified as controls.

So the fix covers every member of globalThis.crypto that had the defect (randomUUID, getRandomValues, and crypto.subtle's 4 KEM methods); everything else in the Web Crypto / node:crypto surface was already correct via the separate bound_native_callable_export_value cache and is unaffected.

Tests

New gap test test-files/test_gap_10427_webcrypto_method_identity.ts, oracle Node 26.5.1. Covers: the issue's own repro (randomUUID === randomUUID, a 3-iteration Set collecting size), getRandomValues identity, the crypto/crypto.subtle namespace objects' own identity (controls), all 4 KEM method identities, 3 already-correct subtle.* methods as controls, cross-read identity (globalThis.crypto.randomUUID === crypto.randomUUID, two different expressions resolving the same property), node:crypto default-vs-named-import identity and randomBytes stability as controls, and a functional sanity check (UUID v4 shape via regex, distinct values across calls, getRandomValues output length) — none of which print an actual random value, so the test is fully deterministic.

Before/after: on the pristine baseline (commit 68a545439), randomUUID stable: false, getRandomValues stable: false, seen.size: 3 (byte-for-byte matching the issue's own "Actual (Perry)" block), and all 4 KEM-method identity lines are false; every already-correct control stayed true. On this branch, output is byte-identical to Node. Harness: PERRY_SKIP_BUILD=1 ./run_parity_tests.sh --filter test_gap_10427 → PARITY_FAIL on the baseline binary, PASS on this branch.

Regression sweep (all still 100% pass, same binary): test_gap_crypto_* (3), test_gap_webcrypto_async_threadpool, test_issue_561_webcrypto, test_crypto_subtle_* (3).

Validation

• cargo test --release -p perry-runtime --lib (RUST_TEST_THREADS=1): 4022 passed, 2 failed, 4 ignored. The 2 failures (gc::tests::copy_slot_decode::sabotaged_remembering_arm_is_refused_by_the_coverage_cross_check, gc::tests::heap_generation::a_free_or_move_outside_every_scope_is_caught_in_debug_builds) are pre-existing on the pristine baseline — confirmed by running the same two tests directly against the unmodified baseline tree (/root/claude-fix-10426-10427-baseline @ 68a545439): identical failures, identical messages. GC internals unrelated to this change (this PR touches none of gc/).

• cargo fmt --all -- --check: clean.

• ./scripts/run_lint_gates.sh (SKIP_COMPILE_GATES=1): 76/77 passed; the one red (Public benchmark evidence freshness) is the pre-existing, repo-wide red documented in the workflow notes — not touched by this change.

• Gap suite: targeted filters above (all pass); full local suite not run (this change is two lines in two small thunk functions, not a hot lowering/runtime path used by most programs).

• Performance: perf stat -e instructions,task-clock, 3 runs each, baseline vs fixed, release binaries (a 2,000,000-iteration loop reading globalThis.crypto.randomUUID as a value each time — the directly-affected path; the same loop reading globalThis.crypto.subtle instead — an already-cached namespace read — as a control):

  program	baseline instructions (avg of 3)	fixed instructions (avg of 3)	Node wall (context)
  crypto.randomUUID read loop (directly affected)	42.75B	31.38B	—
  crypto.subtle read loop (control, unaffected)	27.47B	27.46B	—

  The directly-affected path is ~26.6% fewer instructions, not slower: the baseline paid for a full js_closure_alloc (nursery allocation + GC bookkeeping) on every read; the fix's singleton-cache lookup is a thread-local hashmap probe. The control is unchanged within noise (<0.1%), confirming the fix adds no cost to a path it doesn't touch.

• Package check: not applicable — the issue's repro is a synthetic identity pattern (uuid 14.0.1 was named in the issue's profiling context, not as a reproducible package-level test).

What I did not verify

• The full (non-filtered) gap suite and the 8-shard auto-optimize mode — left to CI's gap-suite shards.
• Whether uuid 14.0.1's actual package source now measurably improves (the issue's ~16%-of-time profiling number was from a 1M-UUID microbenchmark on a real package build, which I didn't reproduce here — the issue itself didn't provide a package-level repro to re-check).
• js_closure_alloc_singleton's cache is unconditionally correct for zero-capture closures per its own doc comment (used elsewhere for the same purpose); I didn't re-audit that mechanism itself, only applied it.

Fixes #10427

Summary by CodeRabbit

• Bug Fixes

  • Web Crypto methods now maintain a stable identity when accessed repeatedly, matching standard behavior.
  • Repeated property reads no longer create unnecessary new function instances.
  • Existing functionality, including UUID generation and random-value retrieval, remains unchanged.

• Tests

  • Added coverage for stable Web Crypto namespaces, methods, imports, and method behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 9eef8850-7ac7-4605-a38e-d8a9f4b193b8

📥 Commits

Reviewing files that changed from the base of the PR and between 68a5454 and cd2cebc.

📒 Files selected for processing (3)

• changelog.d/10643-webcrypto-method-identity.md
• crates/perry-runtime/src/object/global_this/ctor_thunks.rs
• test-files/test_gap_10427_webcrypto_method_identity.ts

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.

---

📝 Walkthrough

Walkthrough

The runtime now caches Web Crypto and SubtleCrypto method closures by function pointer. New tests verify stable identities for namespaces and methods, cross-expression access, node:crypto imports, and method behavior.

Changes

WebCrypto identity

Layer / File(s)	Summary
Use singleton closures crates/perry-runtime/src/object/global_this/ctor_thunks.rs, changelog.d/10643-webcrypto-method-identity.md	Web Crypto and SubtleCrypto method thunks now use singleton closure allocation. The changelog records stable method identity across property reads.
Validate method identity test-files/test_gap_10427_webcrypto_method_identity.ts	Tests cover namespace identity, Web Crypto and KEM method identity, cross-expression access, node:crypto imports, UUID output, and getRandomValues behavior.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to cd2ce

The reported lint concern is unsupported by this repository’s configured checks. The WebCrypto identity change is ready to merge.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The Web Crypto requirement in [#10427] is implemented. webcrypto_method_value and subtle_crypto_method_value now use js_closure_alloc_singleton, and the regression test covers stable identity an…	Implement the [#10554] module-local versus imported-export identity fix, or remove [#10554] from the directly linked requirements if this PR is not intended to address it. Add a regression test for the reported reproduction.
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: caching Web Crypto method closures instead of allocating them on each property read.
Description check	✅ Passed	The description is detailed and covers the summary, root cause, implementation changes, affected APIs, tests, validation results, baseline failures, and linked issue. It does not follow the template h…
Out of Scope Changes check	✅ Passed	The changed runtime code, regression test, and changelog all support [#10427]. The control cases for existing crypto, crypto.subtle, and node:crypto behavior verify that the Web Crypto fix does …

Full details: Linked Issues check

Explanation

The Web Crypto requirement in [#10427] is implemented. webcrypto_method_value and subtle_crypto_method_value now use js_closure_alloc_singleton, and the regression test covers stable identity and functional behavior. The requirement in [#10554] is not implemented by this diff. The PR changes Web Crypto thunk allocation only. It does not change module export/reference lowering or add a test for identity between a module-local function and its imported export.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Landed in merge train 222 (#10732), released as v0.5.1601 — main is now 7c5d04d0ea.

Closing rather than merging is how trains work here: the eight PRs were cherry-picked onto one tree, validated together, and landed under the train's own commit, so GitHub cannot mark this one merged even though your change is on main. git log origin/main will show your commits.

Close-keywords in a source PR body never fire under this scheme, so the issues this train resolved were closed from the train's body instead.

The tree passed: all nine cheap gates, cargo check --workspace --all-targets under -D warnings, the release build of all five pinned artifacts, every unit suite, both derived integration suites, and a 14-area gap sweep with zero unexplained regressions and every area asserted to have run a non-zero number of tests. lint completed its full 6-of-6 compile tier with no failure outside the known-red public-baseline step.