blob: make the Blob__ref/Blob__deref refcount exports unsafe fns

src/jsc/bindings/blob.h

@@ -11,8 +11,8 @@ extern "C" void* Blob__dupe(void* impl);
 extern "C" void* Blob__getDataPtr(JSC::EncodedJSValue blob);
 extern "C" size_t Blob__getSize(JSC::EncodedJSValue blob);
 extern "C" void* Blob__fromBytes(JSC::JSGlobalObject* globalThis, const void* ptr, size_t len);
-extern "C" void* Blob__ref(void* impl);
-extern "C" void* Blob__deref(void* impl);
+extern "C" void Blob__ref(void* impl);
+extern "C" void Blob__deref(void* impl);
 
 // Opaque type corresponding to `bun.webcore.Blob`.
 class BlobImpl;

src/jsc/webcore_types.rs

@@ -232,10 +232,11 @@ impl Blob {
             self.is_heap_allocated(),
             "`finalize` may only be called on a heap-allocated Blob"
         );
-        // `release` returns the raw `m_ctx` pointer without dropping;
-        // `Blob__deref` runs `deinit()` (which `drop(heap::take)`s) when the
-        // count reaches zero.
-        Blob__deref(bun_core::heap::release(self));
+        // SAFETY: `self` is the allocation `Blob::new` produced and the JS
+        // wrapper's `+1` is the count released here. `into_raw` hands the box
+        // over without dropping it; `Blob__deref` runs `deinit()` (which
+        // `drop(heap::take)`s) when the count reaches zero.
+        unsafe { Blob__deref(bun_core::heap::into_raw(self)) }
     }
 
     #[inline]
@@ -471,34 +472,59 @@ impl Blob {
 unsafe impl bun_ptr::ExternalSharedDescriptor for Blob {
     unsafe fn ext_ref(this: *mut Self) {
         // SAFETY: caller guarantees `this` points to a live heap-allocated Blob.
-        unsafe { Blob__ref(&mut *this) }
+        unsafe { Blob__ref(this) }
     }
     unsafe fn ext_deref(this: *mut Self) {
-        // SAFETY: caller guarantees `this` points to a live heap-allocated Blob.
-        unsafe { Blob__deref(&mut *this) }
+        // SAFETY: caller guarantees `this` points to a live heap-allocated Blob
+        // and is releasing a count it owns.
+        unsafe { Blob__deref(this) }
     }
 }
 
+/// Retain half of the refcount protocol behind `BlobImplRefDerefTraits`
+/// (`src/jsc/bindings/blob.h`) and [`bun_ptr::ExternalShared`].
+///
+/// # Safety
+/// `this` must point to a live `Blob` produced by [`Blob::new`], and the call
+/// must happen on the thread that owns it (the count is not atomic). A
+/// by-value `Blob` has a count of zero; bumping it would make a later
+/// [`Blob::deinit`] free an address that was never heap-allocated.
 #[unsafe(no_mangle)]
-pub extern "C" fn Blob__ref(self_: &mut Blob) {
-    debug_assert!(
-        self_.is_heap_allocated(),
-        "cannot ref: this Blob is not heap-allocated"
-    );
-    self_.ref_count.increment();
+unsafe extern "C" fn Blob__ref(this: *mut Blob) {
+    // SAFETY: caller contract above.
+    unsafe {
+        debug_assert!(
+            (*this).is_heap_allocated(),
+            "cannot ref: this Blob is not heap-allocated"
+        );
+        (*this).ref_count.increment();
+    }
 }
 
+/// Release half of the refcount protocol behind `BlobImplRefDerefTraits`
+/// (`src/jsc/bindings/blob.h`), [`bun_ptr::ExternalShared`] and the JS
+/// wrapper's [`Blob::finalize`].
+///
+/// # Safety
+/// `this` must point to a live `Blob` produced by [`Blob::new`], the caller
+/// must own one of its counts (which this call consumes), and the call must
+/// happen on the thread that owns it (the count is not atomic). Releasing the
+/// last count frees the `Blob`, so `this` is dangling once this returns.
 #[unsafe(no_mangle)]
-pub extern "C" fn Blob__deref(self_: &mut Blob) {
-    debug_assert!(
-        self_.is_heap_allocated(),
-        "cannot deref: this Blob is not heap-allocated"
-    );
-    if self_.ref_count.decrement() == bun_ptr::raw_ref_count::DecrementResult::ShouldDestroy {
-        // `deinit` has its own `is_heap_allocated()` guard around the
-        // `drop(heap::take)`, so re-arm so it returns true.
-        self_.ref_count.increment();
-        self_.deinit();
+unsafe extern "C" fn Blob__deref(this: *mut Blob) {
+    // SAFETY: caller contract above. `deinit` frees the allocation, so `this`
+    // is not touched after it.
+    unsafe {
+        debug_assert!(
+            (*this).is_heap_allocated(),
+            "cannot deref: this Blob is not heap-allocated"
+        );
+        if (*this).ref_count.decrement() == bun_ptr::raw_ref_count::DecrementResult::ShouldDestroy {
+            // `deinit` has its own `is_heap_allocated()` guard around the
+            // `drop(heap::take)`, so re-arm so it returns true.
+            (*this).ref_count.increment();
+            (*this).deinit();
+        }
     }
 }
 

src/runtime/webcore/Blob.rs

@@ -107,9 +107,7 @@ pub trait ReadBytesHandler {
 // This crate layers behaviour via the `BlobExt` extension trait below.
 // ──────────────────────────────────────────────────────────────────────────
 
-pub use bun_jsc::webcore_types::{
-    Blob, Blob__deref, Blob__ref, BlobContentType, ClosingState, MAX_SIZE, SizeType,
-};
+pub use bun_jsc::webcore_types::{Blob, BlobContentType, ClosingState, MAX_SIZE, SizeType};
 
 /// 1: Initial
 /// 2: Added byte for whether it's a dom file, length and bytes for `stored_name`,

test/internal/source-lints/unsafe-refcount-exports.test.ts

@@ -0,0 +1,92 @@
+import { file } from "bun";
+import { expect, test } from "bun:test";
+import { realpathSync } from "fs";
+import path from "path";
+import { globAllSources } from "../../../scripts/glob-sources.ts";
+
+// Rust-implemented refcount entry points exported to C++ (`#[unsafe(no_mangle)]`
+// fns named `*__ref`, `*__deref`, `*__unref`, `*__release`) must be `unsafe fn`.
+//
+// Adjusting an intrusive count is only sound when the caller holds a count on
+// an object that is actually refcounted, and no signature can prove that:
+// releasing a count nobody owns frees the object out from under its owner, and
+// bumping the count of a by-value instance turns its ordinary teardown into a
+// free of a non-heap address. That obligation has to be an `unsafe` contract on
+// the export itself, because the same symbol is callable from Rust (the
+// `ExternalSharedDescriptor` impl, finalizers) as well as from the C++
+// `RefDerefTraits` it exists for.
+//
+// Motivating instance: `Blob__ref` / `Blob__deref` in src/jsc/webcore_types.rs
+// were safe `pub extern "C" fn`s over `&mut Blob`, re-exported from
+// `bun_runtime::webcore`, so any safe code holding a `Blob` (stack local,
+// `AnyBlob` payload) could release the count owned by the JS wrapper or an
+// `ExternalShared<Blob>`. `Bun__VmHandle__release` in src/jsc/VmHandle.rs is the
+// shape this lint requires.
+//
+// Scope: definitions only. `safe fn X__deref(..)` declarations of C++-implemented
+// functions inside `unsafe extern "C" { .. }` blocks are a separate population,
+// as are `*__destroy` shims over raw pointers. The suffix list is the
+// enforcement boundary: a new refcount export under another name goes here too.
+//
+// Sibling guards: unsound-erased-box.test.ts, frozen-nonnull-reborrow.test.ts.
+
+const root = path.resolve(import.meta.dir, "..", "..", "..");
+const rustSources = globAllSources().rust.filter(p => p.endsWith(".rs"));
+
+// Only scan files tracked in HEAD (a `git stash` round-trip can leave stray
+// `.rs` files in the working tree; CI runs on a clean checkout). Same guard as
+// dead-code-escapes.test.ts.
+const tracked: Set<string> | null = (() => {
+  const r = Bun.spawnSync({
+    cmd: ["git", "-C", root, "ls-tree", "-r", "--name-only", "-z", "HEAD"],
+    stdout: "pipe",
+    stderr: "ignore",
+  });
+  if (!r.success) return null;
+  return new Set(r.stdout.toString().split("\0").filter(Boolean));
+})();
+
+// `#[unsafe(no_mangle)]`, any further attributes, then the fn header. Group 1 is
+// the `unsafe` qualifier (absent on an offender), group 2 the exported name.
+// Doc comments between the attribute and the header are stripped below.
+const REFCOUNT_EXPORT =
+  /#\[unsafe\(no_mangle\)\]\s*(?:#\[[^\]]*\]\s*)*(?:pub(?:\([^)]*\))?\s+)?(unsafe\s+)?extern\s+"C(?:-unwind)?"\s+fn\s+(\w+__(?:ref|deref|unref|release))\s*\(/g;
+
+const found: string[] = [];
+const offenders: string[] = [];
+let scanned = 0;
+for (const abs of rustSources) {
+  const source = path.relative(root, abs).replaceAll(path.sep, "/");
+  // `src/cli` is a symlink into `src/runtime/cli`; count each file once under
+  // its canonical path.
+  if (path.relative(root, realpathSync(abs)).replaceAll(path.sep, "/") !== source) continue;
+  if (tracked !== null && !tracked.has(source)) continue;
+  scanned++;
+  const content = await file(abs).text();
+  // Strip full-line comments (including `///` docs) so prose mentions don't
+  // count and the attribute -> header match is not interrupted by a doc block.
+  // `[ \t]*`, not `\s*`, so blank lines survive and line numbers stay right.
+  const stripped = content.replace(/^[ \t]*\/\/.*$/gm, "");
+  for (const m of stripped.matchAll(REFCOUNT_EXPORT)) {
+    const line = stripped.slice(0, m.index + m[0].lastIndexOf(m[2])).split("\n").length;
+    const entry = `${source}:${line}: ${m[2]}`;
+    found.push(entry);
+    if (m[1] === undefined) offenders.push(entry);
+  }
+}
+
+test("scans a non-empty set of tracked Rust sources", () => {
+  // Guards against the tracked/realpath filters above over-firing and leaving
+  // nothing to scan, which would make the assertions below pass vacuously.
+  expect(scanned).toBeGreaterThan(0);
+});
+
+test("the pattern still recognizes the tree's refcount exports", () => {
+  // If this goes empty, the exports were renamed or restructured and the
+  // suffix list / regex above needs updating, not the assertion below.
+  expect(found).not.toBeEmpty();
+});
+
+test('exported refcount entry points are declared `unsafe extern "C" fn`', () => {
+  expect(offenders).toEqual([]);
+});