crypto: report external memory usage to GC for node:crypto wrapper classes

[robobun]

What

Wire up reportExtraMemoryAllocated / reportExtraMemoryVisited for the node:crypto C++ wrapper classes that were missing it.

Why

Of the 12 JS-visible wrapper classes in src/jsc/bindings/node/crypto/, only JSDiffieHellman and JSDiffieHellmanGroup reported their external memory to JSC. The rest own heap-allocated OpenSSL contexts and/or arbitrary-length buffers that the GC knew nothing about:

Class	Native data	Size
JSSecretKeyObject	Vector<uint8_t> symmetricKey	arbitrary (user key)
JSPublic/PrivateKeyObject	EVP_PKEY*	key-size dependent
JSHash	EVP_MD_CTX* + digest buffer	arbitrary for XOF (outputLength)
JSHmac	HMAC_CTX*	fixed (~1.3KB)
JSCipher	EVP_CIPHER_CTX*	fixed (~200B)
JSECDH	EC_KEY*	curve-size dependent
JSSign / JSVerify	EVP_MD_CTX*	fixed (~450B)

Without reporting, a loop creating SecretKeyObjects with large keys or shake256 hashes with a large outputLength grows the native heap while the JS heap looks tiny, so GC never kicks in from that pressure.

How

Follows the existing JSDiffieHellman pattern: store m_sizeForGC on the wrapper, call vm.heap.reportExtraMemoryAllocated(this, size) once the native allocation is known, and visitor.reportExtraMemoryVisited(size) in visitChildrenImpl.

• JSKeyObject reports in finishCreation (key data is set at construction). Subclasses inherit via Base::finishCreation / Base::visitChildren.
• JSHash / JSHmac report in init() / initZig() after the ctx is allocated and m_mdLen is known.
• JSSign / JSVerify report in their prototype init after the ctx is stored.
• JSCipher / JSECDH report in finishCreation (ctx/key passed to constructor).

For the opaque OpenSSL contexts, BoringSSL exposes the struct definitions in <openssl/cipher.h> / <openssl/digest.h> / <openssl/hmac.h>, so sizeof(EVP_CIPHER_CTX) etc. are real sizes rather than hardcoded estimates.

Verification

New test at test/js/node/crypto/crypto-extra-memory.test.ts spawns isolated subprocesses that measure heapStats().extraMemorySize before/after allocating N live wrappers and asserts the delta reflects the native memory.

With the released bun, 100 SecretKeyObjects holding 64KB keys each (6.4MB total) bump extraMemorySize by ~78KB. With this change, the delta is >6.4MB.

$ USE_SYSTEM_BUN=1 bun test test/js/node/crypto/crypto-extra-memory.test.ts
 0 pass
 8 fail

$ bun bd test test/js/node/crypto/crypto-extra-memory.test.ts
 8 pass
 0 fail

Existing crypto suites (crypto.test.ts, crypto.key-objects.test.ts, crypto.hmac.test.ts, ecdh.test.ts, sign/verify tests) all pass.

[robobun]

^{Updated 6:08 PM PT - Jun 23rd, 2026}

❌ @robobun, your commit b0fc20b has 1 failures in Build #64345 (All Failures):

• test/js/third_party/grpc-js/test-server.test.ts - SIGTRAP on 🍎 14 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 32653

That installs a local version of the PR into your bun-32653 executable, so you can run:

bun-32653 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7e530a3e-0c24-4c59-bea5-ce1c269834f6

📥 Commits

Reviewing files that changed from the base of the PR and between bfb5184 and 96d75a8.

📒 Files selected for processing (4)

• src/jsc/bindings/node/crypto/JSCipherPrototype.cpp
• src/jsc/bindings/node/crypto/JSHmac.cpp
• src/jsc/bindings/node/crypto/JSSign.cpp
• src/jsc/bindings/node/crypto/JSVerify.cpp

---

Walkthrough

Seven Node.js crypto JSC wrapper classes (JSCipher, JSECDH, JSHash, JSHmac, JSKeyObject, JSSign, JSVerify) gain a m_sizeForGC field to track extra heap memory. Each class reports allocations via vm.heap.reportExtraMemoryAllocated during initialization and reports visited memory via visitor.reportExtraMemoryVisited in GC traversal. Context cleanup operations reset m_sizeForGC to zero when native memory is released. A new test validates the extraMemorySize heap-stat delta for each object type.

Changes

GC Extra-Memory Accounting for Node Crypto Wrappers

Layer / File(s)	Summary
m_sizeForGC field and DECLARE_VISIT_CHILDREN additions src/jsc/bindings/node/crypto/JSCipher.h, JSECDH.h, JSHash.h, JSHmac.h, JSKeyObject.h, JSSign.h, JSVerify.h	Each header adds a size_t m_sizeForGC { 0 } member. JSHash, JSHmac, JSSign, and JSVerify additionally declare DECLARE_VISIT_CHILDREN to enable GC visitation dispatch.
reportExtraMemoryAllocated in init/finishCreation src/jsc/bindings/node/crypto/JSCipher.cpp, JSECDH.cpp, JSHash.cpp, JSHmac.cpp, JSKeyObject.cpp, JSSign.cpp, JSVerify.cpp	Each class's initialization path computes m_sizeForGC from its native context size (sizeof(EVP_CIPHER_CTX), sizeof(EVP_MD_CTX), ECDH group degree bytes, sizeof(HMAC_CTX), or key handle data size) and calls vm.heap.reportExtraMemoryAllocated to register the allocation with the JSC GC.
GC visitation and context cleanup src/jsc/bindings/node/crypto/JSCipher.cpp, JSECDH.cpp, JSHash.cpp, JSHmac.cpp, JSKeyObject.cpp, JSSign.cpp, JSVerify.cpp, JSCipherPrototype.cpp	Each visitChildrenImpl (wired via DEFINE_VISIT_CHILDREN) delegates to Base::visitChildren then calls visitor.reportExtraMemoryVisited(m_sizeForGC), completing the GC accounting round-trip. When native contexts are released (in cipher finalization, HMAC digest, signing, or verification), m_sizeForGC is reset to 0 to remove the GC accounting.
Extra-memory test suite test/js/node/crypto/crypto-extra-memory.test.ts	Adds extraMemoryDelta helper and tests asserting that the JSC extraMemorySize heap-stat increases by at least a fraction of the expected native size when allocating SecretKey, PublicKey/PrivateKey, Hash (XOF), Hmac, Cipher, ECDH, Sign, and Verify objects.

Suggested reviewers

• alii

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly and concisely describes the main change: adding external memory reporting to the GC for node:crypto wrapper classes.
Description check	✅ Passed	The PR description fully covers both required template sections with detailed 'What', 'Why', and 'How' explanations, plus comprehensive verification results.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/jsc/bindings/node/crypto/JSHash.cpp`:
- Around line 132-133: The m_sizeForGC assignment in JSHash only accounts for
m_mdLen but fails to include the memory allocated for m_zigHasher which is owned
and destroyed by JSHash in its destructor. Modify the m_sizeForGC calculation to
add the size of the m_zigHasher allocation alongside m_mdLen so that the garbage
collector accurately tracks the total memory pressure from this object, ensuring
non-XOF Zig hashers do not have invisible native allocations.

In `@src/jsc/bindings/node/crypto/JSHmac.cpp`:
- Around line 103-105: The m_sizeForGC field is set to sizeof(HMAC_CTX) and
memory is reported to the garbage collector, but when the HMAC context is freed
in the digest() method (on both success and failure paths), m_sizeForGC is not
being cleared. This causes finalized Hmac wrappers to continue reporting freed
native memory as allocated. In the digest() method where m_ctx is freed, you
need to also reset m_sizeForGC to 0 and call reportExtraMemoryAllocated again
with a negative or zero value to properly deregister the previously allocated
memory with the garbage collector.

In `@src/jsc/bindings/node/crypto/JSSign.cpp`:
- Around line 211-214: In the signWithKey() method of JSSign, when m_mdCtx is
moved out of the object, the m_sizeForGC tracker is not being reset, causing the
garbage collector to continue reporting memory that no longer belongs to this
JSSign object. After the move operation transfers ownership of m_mdCtx, reset
m_sizeForGC to 0 to ensure accurate memory accounting and prevent the garbage
collector from reporting freed native memory when the Sign object is eventually
finalized.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f7abb350-9a82-4336-84a8-f05ac03dcc53

📥 Commits

Reviewing files that changed from the base of the PR and between d34ca02 and 104da36.

📒 Files selected for processing (15)

• src/jsc/bindings/node/crypto/JSCipher.cpp
• src/jsc/bindings/node/crypto/JSCipher.h
• src/jsc/bindings/node/crypto/JSECDH.cpp
• src/jsc/bindings/node/crypto/JSECDH.h
• src/jsc/bindings/node/crypto/JSHash.cpp
• src/jsc/bindings/node/crypto/JSHash.h
• src/jsc/bindings/node/crypto/JSHmac.cpp
• src/jsc/bindings/node/crypto/JSHmac.h
• src/jsc/bindings/node/crypto/JSKeyObject.cpp
• src/jsc/bindings/node/crypto/JSKeyObject.h
• src/jsc/bindings/node/crypto/JSSign.cpp
• src/jsc/bindings/node/crypto/JSSign.h
• src/jsc/bindings/node/crypto/JSVerify.cpp
• src/jsc/bindings/node/crypto/JSVerify.h
• test/js/node/crypto/crypto-extra-memory.test.ts

[claude]

I didn't find any bugs — the change faithfully follows the existing JSDiffieHellman pattern and has good test coverage — but it touches GC integration across 7 JSC wrapper classes in the crypto bindings, so it's worth a quick human look before merging.

Extended reasoning...

Overview

This PR wires up reportExtraMemoryAllocated / reportExtraMemoryVisited for 7 node:crypto JSC wrapper classes (JSCipher, JSECDH, JSHash, JSHmac, JSKeyObject, JSSign, JSVerify) so the GC sees the native OpenSSL contexts and key buffers they own. Each class gains a size_t m_sizeForGC field, reports it once at allocation/init time, echoes it in visitChildrenImpl, and zeroes it when the native context is released. A new test (crypto-extra-memory.test.ts) asserts heapStats().extraMemorySize grows proportionally for each object type. 16 files changed (15 C++ source/header + 1 test).

Security risks

None identified. The change is purely GC bookkeeping — no cryptographic operations, key handling, or data flow are altered. The size estimates are conservative approximations used only as GC pressure hints; over- or under-reporting affects collection timing, not correctness or confidentiality.

Level of scrutiny

Medium. The implementation is a mechanical application of the existing JSDiffieHellman pattern (verified at JSDiffieHellman.cpp:25-36) and the bug hunter found no issues. However, it spans 7 classes in the JSC bindings and adds new visitChildren implementations that run on GC threads concurrently with mutator writes to m_sizeForGC. While size_t reads/writes are effectively atomic on supported platforms and this matches the established pattern, GC-integration changes in C++ bindings are the kind of thing where a maintainer familiar with Bun's JSC heap conventions should sign off.

Other factors

• CodeRabbit's three minor comments (clear m_sizeForGC on context release for Hmac/Sign/Verify) were addressed in commit 96d75a8; the remaining Zig-hasher comment was withdrawn after a reasonable explanation.
• KeyObjectData is ThreadSafeRefCounted and may be shared across multiple JSKeyObject wrappers, so each will report the full key size — a benign over-report that errs toward more GC pressure.
• No CODEOWNERS cover these paths.
• The new test spawns isolated subprocesses with generous lower-bound thresholds, so it should be stable, but heap-stat-delta tests can occasionally be sensitive to allocator/GC behavior across platforms.

[robobun]

CI status: the diff is green everywhere it ran. The remaining red is unrelated infrastructure and flake across two builds (#64316 and the retrigger #64345):

• :darwin: 26 aarch64 (both shards, both builds): buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun' on runner darwin-aarch64-26-5-1-1. The test runner never started; no tests ran.
• :darwin: 14 aarch64: build 64316 hit test/js/node/test/parallel/test-tls-client-destroy-soon.js (TLS destroySoon byte-count race); build 64345 hit test/js/third_party/grpc-js/test-server.test.ts SIGTRAP. Different tests each run; neither touches src/jsc/bindings/node/crypto/.
• Windows flaky-annotated tests (passed on retry): bun-install, bun-install-registry, bun-install-security-provider, spawn-stdin-readable-stream, dev-and-prod, node-http2-upgrade.

The new test/js/node/crypto/crypto-extra-memory.test.ts and all existing crypto suites pass on every lane that executed. Ready for review.