fix(threading): avoid invalid Channel/FIFO read patterns

[Dicklesworthstone]

What changed

This fixes the audit EXP-033 Channel single-item read shape in bun_threading.

try_read_item() and read_item() previously allocated [MaybeUninit<T>; 1], cast that storage to &mut [T; 1], and then routed through the slice-based read APIs. That fabricates a typed mutable reference before a valid T exists in the slot. For validity-sensitive payloads such as bool, the audit witness reports Miri UB while reading the uninitialized slot:

Undefined Behavior: reading memory ... but memory is uninitialized

The single-item methods now pop directly from the locked FIFO through a shared helper, avoiding the temporary uninitialized T reference entirely. The helper preserves the existing empty/closed-channel behavior and still avoids holding the UnsafeCell buffer borrow across Condition::wait().

During review, I also found a neighboring channel write bug: write_items() appended the entire input slice on every loop iteration while advancing pushed by one. That could duplicate multi-item writes (or return the wrong partial count for fixed buffers). It now writes exactly items[pushed] per successful iteration, matching the loop's accounting and blocking behavior.

Running the new dynamic-buffer multi-item test under Miri then exposed nearby LinearFifo::realign stacked-borrows issues: overlapping copies derived shared raw pointers and mutable raw pointers from the same slice. Both realign copy paths now derive source and destination from one mutable raw pointer.

Test coverage

Added coverage for:

• Channel<bool> single-item reads, so the validity-sensitive payload case is exercised directly under Miri
• dynamic-buffer multi-item writes appending each item exactly once and leaving the channel empty after the expected reads
• wrapped-buffer LinearFifo::realign, which exercises the second raw-copy branch under Miri

Verification

• bun run fmt:rust
• env CARGO_TARGET_DIR=/tmp/bun-channel-fix-target cargo +nightly test -p bun_threading channel::tests
• env CARGO_TARGET_DIR=/tmp/bun-channel-fix-miri-target cargo +nightly miri test -p bun_threading channel::tests
• env CARGO_TARGET_DIR=/tmp/bun-channel-fix-target cargo +nightly check -p bun_threading
• env CARGO_TARGET_DIR=/tmp/bun-channel-fix-target cargo +nightly test -p bun_collections linear_fifo
• env CARGO_TARGET_DIR=/tmp/bun-channel-fix-miri-target cargo +nightly miri test -p bun_collections linear_fifo_realign_wrapped_buffer
• env CARGO_TARGET_DIR=/tmp/bun-channel-fix-target cargo +nightly check -p bun_collections
• git diff --check
• env CARGO_TARGET_DIR=/tmp/bun-channel-clean-target cargo +nightly check --workspace from a clean HEAD snapshot

The workspace check only emitted existing warnings.

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[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: a8f261e1-dbcb-4b46-ba4e-19a2c8886680

📥 Commits

Reviewing files that changed from the base of the PR and between 5a63c2b and b471ab4.

📒 Files selected for processing (2)

• src/collections/linear_fifo.rs
• src/threading/channel.rs

---

Walkthrough

Refactors Channel read paths by adding a mutex-guarded helper read_item_locked() used by try_read_item, read_item, and read_items; updates write loop to call per-item write_item; refactors LinearFifo realign pointer copy for wrapped/non-wrapped paths; removes MaybeUninit import and adds tests.

Changes

Channel read path consolidation

Layer / File(s)	Summary
Locked-read helper and single-item API refactor src/threading/channel.rs	read_item_locked() added to centralize mutex-guarded empty/closed checks and unsafe buffer reads. try_read_item acquires the mutex and returns the helper result directly. read_item loops, calling the helper and waiting on getters until an item or close. Removed MaybeUninit import.
Bulk read and write adjustments src/threading/channel.rs	read_items loop now calls read_item_locked() to obtain each item instead of duplicating read/closed logic. write_items now calls the single-item write_item per iteration.
LinearFifo realign pointer copy src/collections/linear_fifo.rs	Non-wrapping realignment now uses a raw *mut T buffer pointer and ptr::copy from buf.add(head) to buf. Wrapped realign path reuses raw buf pointer for scratch ↔ buf byte copies and overlapping shift; added a test exercising wrapped-buffer realign.
Tests for single/multi-item behavior src/threading/channel.rs, src/collections/linear_fifo.rs	New/updated unit tests: verify Channel<bool> single-item try_read_item/read_item behavior and Closed after close(), validate multi-item writes/reads for a Channel<u8> using a dynamic buffer, and test LinearFifo::realign when data wraps.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(threading): avoid invalid Channel/FIFO read patterns' directly and clearly summarizes the main changes: fixing undefined behavior in Channel read methods and FIFO realignment by avoiding invalid read patterns.
Description check	✅ Passed	The description provides comprehensive details under non-template sections, explaining the audit fix, bugs found/fixed, and verification steps performed, though it doesn't use the repository's required template structure.
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.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 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/collections/linear_fifo.rs`:
- Around line 283-285: The current DynamicBuffer<T>::as_mut_slice() (via
assume_init_slice_mut()) creates &mut [T] over uninitialized memory which is UB
for non‑"any‑bit‑pattern" types like bool; update the buffer API used by
LinearFifo and Channel::init_dynamic() to avoid creating typed references to
uninitialized memory: either add a trait bound (e.g., require T: Pod) on
DynamicBuffer/LinearFifo/Channel so assume_init_slice_mut() is only used for POD
types, or change the trait to expose &mut [MaybeUninit<T>] / raw pointers to
MaybeUninit<T> and perform unsafe copies/rotations with ptr::copy /
ptr::copy_nonoverlapping on the MaybeUninit storage (e.g., replace calls to
as_mut_slice() and buf.as_mut_slice().as_mut_ptr() with MaybeUninit-aware
operations) so no &mut T is formed for uninitialized slots.

🪄 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: 3d349eaa-9682-4315-aed3-f72a76321044

📥 Commits

Reviewing files that changed from the base of the PR and between 620f98d and 5a63c2b.

📒 Files selected for processing (2)

• src/collections/linear_fifo.rs
• src/threading/channel.rs