blob: make the Blob__ref/Blob__deref refcount exports unsafe fns

Merged
RO
3mo ago
+144-28

Problem

Blob__ref and Blob__deref in src/jsc/webcore_types.rs are the intrusive refcount entry points behind BlobImplRefDerefTraits (src/jsc/bindings/blob.h) and the ExternalSharedDescriptor for Blob impl. They were declared as safe functions:

#[unsafe(no_mangle)]
pub extern "C" fn Blob__ref(self_: &mut Blob)
#[unsafe(no_mangle)]
pub extern "C" fn Blob__deref(self_: &mut Blob)

and bun_runtime::webcore re-exported them. Blob is an ordinary struct that also lives by value (AnyBlob payloads, stack locals, Blob::dupe() results), so a &mut Blob is easy to come by in safe code, and both functions have preconditions that nothing in the signature proves:

  • Blob__deref releases a count the caller must own. On a heap Blob it double frees against the ExternalShared<Blob> or JS wrapper that actually owns the count (deref runs deinit(), which heap::takes the allocation). On a by-value Blob it underflows the count (debug_assert!(is_heap_allocated()) in debug builds).
  • Blob__ref requires a heap Blob. is_heap_allocated() is encoded as ref_count != 0, so bumping a by-value Blob from 0 to 1 makes its ordinary deinit() run heap::take on an address that was never boxed. This is why the retain side needs the contract too, unlike e.g. Bun__VmHandle__retain(&VirtualMachine), where the reference already proves everything the function needs.

No in-tree Rust caller misuses them today; this is a contract fix. Same class as the other "safe fn that releases something it cannot prove the caller owns" fixes (#31981, #30812).

Fix

  • Both are now private unsafe extern "C" fns over *mut Blob with the ownership and threading contract in a # Safety section. This is the shape of ExternalSharedDescriptor::ext_ref/ext_deref (so the impl forwards the pointer as is) and of the other teardown exports in the crate (AbortSignal__Timeout__deinit, Bun__VmHandle__release). Taking a raw pointer rather than materializing &mut Blob at the FFI boundary also follows the provenance guidance in src/CLAUDE.md for entry points that may free their argument; the bodies are unchanged otherwise. #[unsafe(no_mangle)] keeps the symbols exported, so blob.h and the C++ callers are unaffected; per the workspace pub convention the items no longer need to be pub since no other crate imports them.
  • The two Rust callers (ExternalSharedDescriptor impl, Blob::finalize) now acknowledge the contract with unsafe blocks. finalize hands its Box over with heap::into_raw, which is what Blob__deref -> deinit -> heap::take pairs with.
  • The dead re-export of both names from src/runtime/webcore/Blob.rs is removed.
  • blob.h declared both as returning void*; the definitions return nothing. C++ ignores the value, so this was harmless, but the declarations now match.

Intentionally not touched here, same area, different fixes: Blob::deinit(&mut self) still frees the allocation itself when ref_count != 0 (it is the teardown Blob__deref and the structured-clone error path rely on, so changing it means auditing its callers), and the safe fn X__deref(..) declarations of C++-implemented refcount functions in unsafe extern "C" blocks are a separate population.

Test

test/internal/source-lints/unsafe-refcount-exports.test.ts scans the tracked Rust sources for #[unsafe(no_mangle)] exports named *__ref, *__deref, *__unref or *__release and requires them to be declared unsafe. It also asserts the pattern still finds the tree's refcount exports (currently Blob__ref, Blob__deref, Bun__VmHandle__release), so it cannot pass vacuously after a rename. On main it fails with:

+   "src/jsc/webcore_types.rs:483: Blob__ref",
+   "src/jsc/webcore_types.rs:492: Blob__deref",

The compiler enforces the rest: reverting either signature to safe trips unused_unsafe (warnings are denied workspace-wide) at the two Rust call sites.

Verification

  • bun bd builds clean; bun bd test test/internal/source-lints/unsafe-refcount-exports.test.ts passes, and fails with the output above with src/ stashed.
  • bun test test/internal/source-lints/ (all 17 lints) passes; cargo fmt -p bun_jsc -p bun_runtime -- --check clean.
  • Suites that drive the C++ side of the protocol (RefPtr<Blob> in FormData, File and WebSocket) on the debug build: test/js/web/html/FormData.test.ts, FormData-file-error-leak.test.ts, test/js/web/fetch/blob.test.ts, test/js/web/websocket/websocket-blob.test.ts pass. test/js/web/fetch/blob-file-name-ownership.test.ts (4000 structuredClone round-trips, each finalized through Blob__deref) takes about 4s under the debug build on this machine and so sits at its default 5s timeout; it passes with a longer timeout, and its subprocess prints the expected output with no ASAN report.