blob: hand the read handler's pointer to on_read_bytes instead of &mut self

[robobun]

What

ReadBytesHandler::on_read_bytes (src/runtime/webcore/Blob.rs) took &mut self. Its only implementor, BlobReadChain in src/runtime/image/Image.rs, used that receiver to reclaim the Box it lives in:

fn on_read_bytes(&mut self, result: ReadBytesResult) {
    let boxed = unsafe { bun_core::heap::take(std::ptr::from_mut::<Self>(self)) };
    boxed.on_read_bytes_impl(result);   // Box freed here, while `&mut self` is still on the stack
}

This is the Bun.Image source path for Bun.file() / Bun.s3() / zero-length in-memory Blobs. No crash is known; it is an aliasing-model violation (latent UB), found while converting Blob::deinit (#37672), which has the same shape.

Why it is wrong

A reference argument is protected for the whole call under both aliasing models, and deallocating protected memory is UB even if the reference is never used again. bun_runtime cannot run under Miri, so here is a reduction with exactly this shape (a trait method on_read_bytes(&mut self) that Box::from_raws its receiver, dispatched as H::on_read_bytes(unsafe { &mut *ctx }, ..) like the sites in read_bytes_to_handler), next to the shape this PR switches to:

# Tree Borrows (the flags `bun run rust:miri` uses)
test tests::by_ptr_receiver ... ok
test tests::by_ref_receiver ... error: Undefined Behavior: deallocation through <144833> at alloc47733[0x0] is forbidden
     = help: the allocation of the accessed tag <144833> also contains the strongly protected tag <144822>
     = help: the strongly protected tag <144822> disallows deallocations
  18 |         let boxed = unsafe { Box::from_raw(std::ptr::from_mut::<Self>(self)) };
help: the strongly protected tag <144822> was created here, in the initial state Reserved
  17 |     fn on_read_bytes(&mut self, bytes: Vec<u8>) {

# Stacked Borrows
test tests::by_ref_receiver ... error: Undefined Behavior: deallocating while item [Unique for <131080>] is strongly protected
test tests::by_ptr_receiver ... ok

Fix

The trait method becomes unsafe fn on_read_bytes(this: *mut Self, result), the shape ReadFileCompletion::run(ctx: *mut Self, ..) in blob/read_file.rs already uses for the same job: read_bytes_to_handler passes the ctx it was given straight through at its four delivery sites (file completion, file cancel, S3 callback, synchronous in-memory), and the Image impl does heap::take(this). The only frame on the stack when the Box is freed now holds a raw pointer, which is what both models allow. No behaviour change; the pointer value delivered is the same one as before.

The handoff also asked whether BlobReadChain::start leaks the chain when read_bytes_to_handler returns Err. It does not: the only Err source is the S3 branch, and execute_simple_s3_request only returns Err when the callback itself did, i.e. after it has already delivered to (and consumed) the handler; the file branch is infallible and the in-memory branch delivers before returning. That "exactly one delivery, whatever the return value" contract is what the ownership transfer relies on, so it is now written down on read_bytes_to_handler instead of being implicit.

Tests

• test/internal/source-lints/self-receiver-reclaim.test.ts pins the shape tree-wide (heap::take / heap::destroy / Box::from_raw of self or a pointer spelled from self). It fails on main with src/runtime/image/Image.rs:1385: heap::take(std::ptr::from_mut::<Self>(self) and passes with this change. Blob::deinit in src/jsc/webcore_types.rs is the one other instance; it is allowlisted with a ratchet and is being converted in blob: delete Blob::deinit, which freed the allocation through &mut self #37672. That PR adds the same lint file with the allowlist the other way round, so whichever of the two lands second drops the remaining allowlist entry (the ratchet test says so when it happens); the two PRs do not otherwise touch the same lines.
• test/js/bun/image/image.test.ts: new case for an S3-backed source (local Bun.serve endpoint, success and NoSuchKey) and a zero-length in-memory slice (synchronous delivery), the two deliveries the existing Bun.file() test does not reach. The S3 client applies an ambient HTTP_PROXY regardless of NO_PROXY (S3Client ignores NO_PROXY when HTTP_PROXY is set #32045), so the test blanks the proxy variables around the S3 calls.
• test/js/web/workers/worker-refused-completion.test.ts: new row with an image read in flight when the worker goes away, so the cancel delivery (handler told ECANCELED, frees itself) runs under the debug/ASAN build.

Debug (ASAN) build: image.test.ts 94 pass, image-adversarial.test.ts 61 pass (the concurrent BlobReadChain case included), worker-refused-completion.test.ts 16 pass, test/internal/source-lints/ 82 pass; cargo clippy -p bun_runtime clean on the touched files.

[robobun]

^{Updated 5:35 PM PT - Aug 11th, 2026}

✅ @robobun, your commit 039d46e3cc682910bebefa0e6c4dd13a58495afa passed in Build #92643! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37681

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

bun-37681 --bun

[robobun]

Status: reproduced the shape with a standalone Miri reduction (Tree Borrows and Stacked Borrows both reject the &mut self receiver freeing its own Box; the raw-pointer receiver passes, output in the description). The in-tree instance is pinned by test/internal/source-lints/self-receiver-reclaim.test.ts, which fails on main at src/runtime/image/Image.rs:1385 and passes here. Coordinates with #37672, which adds the same lint with the allowlist the other way round; whichever lands second drops the leftover allowlist entry.

[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: 74c0b5fd-598a-4deb-a06b-78bcf20b4b13

📥 Commits

Reviewing files that changed from the base of the PR and between ae33ace and 039d46e.

📒 Files selected for processing (5)

• src/runtime/image/Image.rs
• src/runtime/webcore/Blob.rs
• test/internal/source-lints/self-receiver-reclaim.test.ts
• test/js/bun/image/image.test.ts
• test/js/web/workers/worker-refused-completion.test.ts

---

Walkthrough

Changes

Blob read ownership

Layer / File(s)	Summary
Callback contract and delivery paths src/runtime/webcore/Blob.rs	ReadBytesHandler now receives an unsafe raw pointer. Synchronous, file, cancellation, and S3 paths consume the context exactly once.
Image read-chain reclamation src/runtime/image/Image.rs	BlobReadChain passes the leaked pointer directly to the dispatcher. The callback reclaims it before processing the result.
Ownership lint and image coverage test/internal/source-lints/self-receiver-reclaim.test.ts, test/js/bun/image/image.test.ts, test/js/web/workers/worker-refused-completion.test.ts	The lint detects receiver reclamation patterns. Tests cover S3, Blob, empty data, missing objects, and worker refusal paths.

Possibly related PRs

• oven-sh/bun#37625: Both PRs modify Blob.rs and address Blob read-chain ownership documentation or behavior.

Suggested reviewers: jarred-sumner, dylan-conway

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary change from a protected mutable reference to a raw pointer in the read handler callback.
Description check	✅ Passed	The description explains the problem, fix, ownership contract, affected paths, and verification results, although its headings differ from the template.
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.

---

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

[claude]

LGTM — a targeted receiver-type change (&mut self → this: *mut Self) that fixes a real aliasing-model violation without changing runtime behavior.

What was reviewed

• Confirmed BlobReadChain is the only ReadBytesHandler implementor and all four read_bytes_to_handler delivery sites (file call/cancel, S3 callback, in-memory) now hand the raw ctx through unchanged — same pointer value as before, just no protected &mut on the stack when the Box is freed.
• Checked the new source-lint follows the sibling lints' conventions (globAllSources().rust, tracked-file filter, ratchet allowlist, regex self-test with positive and negative fixtures) and that the allowlisted src/jsc/webcore_types.rs entry coordinates cleanly with #37672.
• New image test is hermetic (local Bun.serve S3 endpoint, proxy env restored in finally); worker-refused-completion row slots into the existing table.

Extended reasoning...

Overview

The PR changes ReadBytesHandler::on_read_bytes in src/runtime/webcore/Blob.rs from taking &mut self to unsafe fn on_read_bytes(this: *mut Self, ..), updates the four call sites in read_bytes_to_handler to pass the raw ctx pointer directly instead of &mut *ctx, and updates the sole implementor (BlobReadChain in src/runtime/image/Image.rs) to heap::take(this) instead of heap::take(ptr::from_mut(self)). It also adds a tree-wide source-lint pinning the shape, an S3/zero-length image test covering the two previously-unexercised delivery paths, and a worker-teardown row for the cancel path.

Security risks

None. This is a receiver-type refactor on an internal trait; no user-facing API surface, no parsing of untrusted input, no auth/crypto/permission code touched. The pointer value delivered to the handler is byte-identical to before.

Level of scrutiny

Medium — it touches unsafe FFI/heap code, but the change is mechanical and follows the codebase's own documented pattern for exactly this hazard (src/CLAUDE.md's "Pointer provenance at FFI boundaries" section, and the existing ReadFileCompletion::run(ctx: *mut Self, ..) shape the trait doc now points at). The PR description includes a Miri reduction confirming both Tree Borrows and Stacked Borrows reject the old shape and accept the new one.

Other factors

• The trait has exactly one implementor (grep-verified), so the whole class is fixed here.
• The documented "exactly one delivery, whatever this returns" contract on read_bytes_to_handler is not new behavior — it was already what BlobReadChain::start relied on; the PR writes it down. If that contract were ever wrong it would be a pre-existing bug, not one introduced here.
• The source-lint is defensive rather than load-bearing: it has a self-test asserting both banned and allowed spellings, a non-empty-scan guard against vacuous passes, and a ratchet so the one allowlisted instance can't silently multiply.
• Tests are hermetic (local Bun.serve for S3, proxy env vars saved/restored in finally) and cover the S3 success/error arms, the synchronous in-memory arm, and the worker-cancel arm — the three delivery paths the existing Bun.file() test didn't reach.