strings: bind StringJoiner borrowed segments to a lifetime (#31493)
`StringJoiner` stored every borrowed segment as a lifetime-erased raw
fat pointer (`RawSlice<u8>` plus an `owns_slice` flag), with blanket
`unsafe impl Send/Sync` on both the joiner and its nodes and a manual
`Drop` to free owned slices. Nothing tied a pushed segment to the data
it pointed at, so every `push()` call site relied on an undocumented
"must outlive `done()`" convention.
### What changed
- `StringJoiner` and `Node` take a lifetime parameter; `Node` is now
`enum { Borrowed(&'a [u8]), Owned(Box<[u8]>) }`. This deletes the manual
`Drop`, the four `unsafe impl Send/Sync`, the `RawSlice` round-trips,
and `Watcher.input`'s `&'static` workaround.
- `push()` / `push_static()` borrow segments for the joiner's lifetime,
so the sourcemap join (`append_source_map_chunk`), the dev-server
sourcemap store, and most bundler/Blob/FormData call sites are now plain
borrow-checked code.
- The bundler's deferred path stores the joiner in
`Chunk.intermediate_output` (no lifetime parameter), so each of the four
`break_output_into_pieces` callers (`postProcessJSChunk`,
`postProcessCSSChunk`, `postProcessHTMLChunk`,
`MetafileBuilder::generate`) re-tags the joiner with a new documented
`unsafe StringJoiner::detach_lifetime()` immediately before the call,
with a SAFETY comment naming exactly which storage the borrowed nodes
point into and why it outlives the stored joiner.
- Blob constructor / FormData serialization keep borrowing (not copying)
buffers whose liveness comes from GC rooting, via
`bun_ptr::detach_lifetime` with per-site SAFETY comments — including the
FormData file-entry `Data::Bytes` site and the top-level typed-array
arm, each with its own justification.
- Fixes a latent use-after-free in `postProcessJSChunk`'s
`InternalBakeDev` arm: it pushed the bytes of a block-local
`MutableString` by reference into a joiner that is stored in
`chunk.intermediate_output` and read later. The bytes are now moved into
the joiner (`take_slice()` + `push_owned`).
- Adds unit tests for the node-storage rewrite.
### Net unsafe accounting
Removed: 4 blanket `unsafe impl Send/Sync` (which made every push call
site unauditable), the `unsafe` free in `Node::drop`, the `unsafe`
`RawSlice::from_raw` in `push_owned`, and the lifetime-erased `RawSlice`
storage itself.
Added: one `unsafe fn detach_lifetime()` on `StringJoiner`, 4
`detach_lifetime()` call sites in the bundler, and 6
`bun_ptr::detach_lifetime` slice re-tags in Blob/FormData — 10
narrowly-scoped expressions, each with a SAFETY comment naming the owner
of the bytes and why it outlives the joiner's last read.
The trade is deliberate: instead of two type-level `unsafe impl`s that
silence the compiler for every present and future call site, the
remaining unsafety is at the specific points where the borrow genuinely
cannot be expressed.
### Why lifetimes alone can't cover the remaining sites
- **Chunk self-reference:** the joiner stored in
`Chunk.intermediate_output` borrows from the same chunk's
`compile_results_for_chunk` (and from linker-graph data). A struct field
cannot name a lifetime that refers to a sibling field of the same
struct, so expressing this with pure lifetimes would mean copying every
compiled segment at join time or restructuring chunk storage.
- **GC-kept-alive Blob/FormData buffers:** the pushed bytes are owned by
JS-heap objects (ArrayBuffers, Blob stores, `DOMFormData` entries) whose
liveness over the push→`done()` span is guaranteed by GC rooting, not by
any Rust owner with a nameable lifetime. Borrow-checking those would
require cloning each buffer; the prior code already borrowed them, so
this keeps the no-copy behavior with the justification written down.
### Testing
- `cargo check -p bun_bin`, `bun run rust:check-all` (10/10 targets),
debug build smoke test.
- Bundler: `bundler_edgecase`, `bundler_npm`, `bundler_minify`,
`bundler_splitting` — all pass.
- Bake dev server: `test/bake/dev/esm.test.ts`,
`test/bake/dev/html.test.ts` — pass.
- FormData/Blob: `FormData.test.ts`,
`FormData-multipart-serialization.test.ts`, `blob.test.ts` — pass except
two pre-existing debug-build failures (a 5s-timeout GC test and an
RSS-threshold test) that fail identically on a main debug build.
- Sourcemaps: `test/js/bun/sourcemap/`,
`compile-sourcemap-internal.test.ts` — pass. The
`bun-build-compile*.test.ts` standalone-executable tests time out at 5s
when spawning debug-build binaries on this machine; the same tests fail
identically on a main debug build (0 new failures).
- New `StringJoiner` unit tests compile under `cargo check`; `cargo test
-p bun_core` does not link standalone on main either (pre-existing). J
Jarred Sumner committed
f20c8dc33a978907bb0c70f3609011fa1645dd9e
Parent: 472a06a
Committed by GitHub <noreply@github.com>
on 5/28/2026, 4:21:51 AM