valkey: use VecDeque for the command queues; LinearFifo rejects droppable items

src/collections/linear_fifo.rs

@@ -1,5 +1,6 @@
-// FIFO of fixed size items
-// Usually used for e.g. byte buffers
+// FIFO of fixed size `Copy` items, usually byte buffers, raw pointers or small
+// plain structs. The ring never runs item destructors, so `T: Copy` is the
+// contract; owning element types belong in `VecDeque`.
 
 use core::marker::PhantomData;
 use core::mem::{self, MaybeUninit};
@@ -35,8 +36,7 @@ pub trait LinearFifoBuffer<T> {
 /// layout to `T`; exposing uninitialized bytes as `T` is sound only when any
 /// bit pattern is a valid `T`. NOT every in-tree element type satisfies this:
 /// besides byte buffers and raw pointers, fifos today store `NonNull`-bearing
-/// enums (`bun_test::RefDataValue`), `JSPromiseStrong`-bearing structs
-/// (`ValkeyCommand::PromisePair`), and the `event_loop::Task` enum — see the
+/// enums (`bun_test::RefDataValue`) and the `event_loop::Task` enum — see the
 /// `StaticBuffer` note below for the pending MaybeUninit accessor rework.
 /// Centralises the four per-buffer-kind casts behind one audited block.
 #[inline(always)]
@@ -54,22 +54,14 @@ fn assume_init_slice_mut<T>(s: &mut [MaybeUninit<T>]) -> &mut [T] {
     unsafe { &mut *(ptr::from_mut::<[MaybeUninit<T>]>(s) as *mut [T]) }
 }
 
-/// Shift `slice[1..]` down to `slice[0..len-1]` (memmove). Used by
-/// `ordered_remove_item` for the four wrap/non-wrap segment shifts. Not
-/// `slice::copy_within` because that requires `T: Copy`; this fifo permits
-/// move-only `T` (the duplicated tail slot is logically discarded by the
-/// subsequent `count -= 1`).
+/// Shift `slice[1..]` down to `slice[0..len-1]`. Used by
+/// `ordered_remove_item` for the four wrap/non-wrap segment shifts; the
+/// duplicated tail slot is logically discarded by the subsequent `count -= 1`.
 #[inline(always)]
-fn shift_down_one<T>(slice: &mut [T]) {
-    let len = slice.len();
-    if len <= 1 {
-        return;
-    }
-    let p = slice.as_mut_ptr();
-    // SAFETY: src `[1..len)` and dst `[0..len-1)` are both in-bounds of
-    // `slice`; `ptr::copy` handles the overlap. Both pointers derive from one
-    // `as_mut_ptr()` so the src tag is not invalidated by a later Unique retag.
-    unsafe { ptr::copy(p.add(1), p, len - 1) };
+fn shift_down_one<T: Copy>(slice: &mut [T]) {
+    if slice.len() > 1 {
+        slice.copy_within(1.., 0);
+    }
 }
 
 #[cfg(debug_assertions)]
@@ -95,14 +87,14 @@ fn poison<T>(slice: &mut [T], n: usize) {
 // (`writable_slice` hands out `&mut [T]` over not-yet-written slots) bakes in
 // the same exposure for every buffer kind. Sound only for `T` whose
 // any-bit-pattern is valid — and in-tree element types ALREADY violate that:
-// `RefDataValue` (NonNull<DescribeScope> payload), `PromisePair`
-// (JSPromiseStrong), and the `Task` enum are stored in fifos today, so
-// materialising `&[T]` over uninitialized slots for those types is latent UB.
+// `RefDataValue` (NonNull<DescribeScope> payload) and the `Task` enum are
+// stored in fifos today, so materialising `&[T]` over uninitialized slots for
+// those types is latent UB.
 // The fix is reworking the accessors to operate on `&[MaybeUninit<T>]` and
-// only assume-init the logically-written subranges. That cannot be done by
-// touching this file alone — `writable_slice`-family callers in other crates
-// see the signature change — so it is deferred to a dedicated change with
-// Miri coverage for a NonNull-bearing element type.
+// only assume-init the logically-written subranges; `writable_slice`-family
+// callers in other crates see the signature change, so that lives in #31835.
+// The `T: Copy` bound below only pins the no-destructor contract; it does not
+// close this gap.
 pub struct StaticBuffer<T, const N: usize>([MaybeUninit<T>; N]);
 
 impl<T, const N: usize> LinearFifoBuffer<T> for StaticBuffer<T, N> {
@@ -158,7 +150,7 @@ pub struct LinearFifo<T, B: LinearFifoBuffer<T>> {
 // re-exported as `bun_io::Write`), plus `std::io::Read`, `std::io::Write`,
 // and `core::fmt::Write` for std interop.
 
-impl<T, const N: usize> LinearFifo<T, StaticBuffer<T, N>> {
+impl<T: Copy, const N: usize> LinearFifo<T, StaticBuffer<T, N>> {
     /// `init` for `.Static`.
     pub fn init() -> Self {
         Self {
@@ -170,7 +162,7 @@ impl<T, const N: usize> LinearFifo<T, StaticBuffer<T, N>> {
     }
 }
 
-impl<T> LinearFifo<T, DynamicBuffer<T>> {
+impl<T: Copy> LinearFifo<T, DynamicBuffer<T>> {
     /// `init` for `.Dynamic`.
     pub fn init() -> Self {
         Self {
@@ -182,10 +174,10 @@ impl<T> LinearFifo<T, DynamicBuffer<T>> {
     }
 }
 
-// `pub fn deinit` → Drop. Dynamic frees `buf` via `Box` drop; Static/Slice are
-// no-ops. Field drop glue covers it; no explicit impl needed.
+// `pub fn deinit` → Drop. Dynamic frees `buf` via `Box` drop; Static is a
+// no-op. Items are never dropped, which is why every impl requires `T: Copy`.
 
-impl<T, B: LinearFifoBuffer<T>> LinearFifo<T, B> {
+impl<T: Copy, B: LinearFifoBuffer<T>> LinearFifo<T, B> {
     #[inline]
     fn buf_len(&self) -> usize {
         self.buf.len()
@@ -394,10 +386,7 @@ impl<T, B: LinearFifoBuffer<T>> LinearFifo<T, B> {
     }
 
     /// Read data from the fifo into `dst`, returns number of items copied.
-    pub(crate) fn read(&mut self, dst: &mut [T]) -> usize
-    where
-        T: Copy,
-    {
+    pub(crate) fn read(&mut self, dst: &mut [T]) -> usize {
         let total = dst.len();
         let mut dst_left = &mut dst[..];
 
@@ -466,10 +455,7 @@ impl<T, B: LinearFifoBuffer<T>> LinearFifo<T, B> {
 
     /// Appends the data in `src` to the fifo.
     /// You must have ensured there is enough space.
-    pub(crate) fn write_assume_capacity(&mut self, src: &[T])
-    where
-        T: Copy,
-    {
+    pub(crate) fn write_assume_capacity(&mut self, src: &[T]) {
         debug_assert!(self.writable_length() >= src.len());
 
         let mut src_left = src;
@@ -503,19 +489,15 @@ impl<T, B: LinearFifoBuffer<T>> LinearFifo<T, B> {
             tail %= self.buf_len();
         }
         // SAFETY: `tail` is in-bounds (capacity reserved by caller). The slot is
-        // logically uninitialized — `ptr::write` does not drop the prior
-        // bit-pattern, which is required for non-`Copy` `T` whose backing
-        // storage is `MaybeUninit<T>`.
+        // logically uninitialized `MaybeUninit<T>` storage; `ptr::write`
+        // initializes it without reading the prior bit-pattern.
         unsafe { ptr::write(self.buf.as_mut_slice().as_mut_ptr().add(tail), item) };
         self.update(1);
     }
 
     /// Appends the data in `src` to the fifo.
     /// Allocates more memory as necessary
-    pub fn write(&mut self, src: &[T]) -> Result<(), AllocError>
-    where
-        T: Copy,
-    {
+    pub fn write(&mut self, src: &[T]) -> Result<(), AllocError> {
         self.ensure_unused_capacity(src.len())?;
         self.write_assume_capacity(src);
         Ok(())
@@ -536,10 +518,7 @@ impl<T, B: LinearFifoBuffer<T>> LinearFifo<T, B> {
     }
 
     /// Place data back into the read stream
-    pub fn unget(&mut self, src: &[T]) -> Result<(), AllocError>
-    where
-        T: Copy,
-    {
+    pub fn unget(&mut self, src: &[T]) -> Result<(), AllocError> {
         self.ensure_unused_capacity(src.len())?;
 
         self.rewind(src.len());
@@ -561,10 +540,7 @@ impl<T, B: LinearFifoBuffer<T>> LinearFifo<T, B> {
 
     /// Returns the item at `offset`.
     /// Asserts offset is within bounds.
-    pub fn peek_item(&self, offset: usize) -> T
-    where
-        T: Copy,
-    {
+    pub fn peek_item(&self, offset: usize) -> T {
         debug_assert!(offset < self.count);
 
         let mut index = self.head + offset;

src/install/lockfile/Tree.rs

@@ -1073,6 +1073,7 @@ impl Tree {
 // FillItem / TreeFiller
 // ──────────────────────────────────────────────────────────────────────────
 
+#[derive(Clone, Copy)]
 pub struct FillItem {
     pub(crate) tree_id: Id,
     pub(crate) dependency_id: DependencyID,

src/runtime/test_runner/bun_test.rs

@@ -777,9 +777,9 @@ impl BunTest {
             return Ok(());
         }
 
-        this.add_result(refdata.phase.clone());
+        this.add_result(refdata.phase);
         // `this` borrow ends here (NLL); `run_next_tick` re-derives via `.get()`.
-        Self::run_next_tick(&refdata.buntest_weak, global_this, refdata.phase.clone());
+        Self::run_next_tick(&refdata.buntest_weak, global_this, refdata.phase);
         Ok(())
     }
 
@@ -852,8 +852,8 @@ impl BunTest {
         };
         // SAFETY: `&mut` derived via `UnsafeCell`; borrow ends before
         // `run_next_tick` re-derives.
-        strong.get().add_result(ref_in.phase.clone());
-        Self::run_next_tick(&ref_in.buntest_weak, global_this, ref_in.phase.clone());
+        strong.get().add_result(ref_in.phase);
+        Self::run_next_tick(&ref_in.buntest_weak, global_this, ref_in.phase);
 
         Ok(JSValue::UNDEFINED)
     }
@@ -1201,7 +1201,7 @@ impl BunTest {
                 if unsafe { (*dcb_data).called } {
                     // done callback already called or the callback errored; add result immediately
                 } else {
-                    let r = Self::ref_(this_strong, cfg_data.clone());
+                    let r = Self::ref_(this_strong, cfg_data);
                     let alias = NonNull::new(r.as_ptr())
                         .expect("ref_() returns a freshly-boxed RefData");
                     // SAFETY: see above. Move the sole +1 into the DoneCallback.
@@ -1421,10 +1421,10 @@ pub struct EntryData {
     pub(crate) remaining_repeat_count: i64,
 }
 
-// Clone: bitwise OK — `active_scope` is a non-owning borrow of a
+// Clone/Copy: bitwise OK — `active_scope` is a non-owning borrow of a
 // `DescribeScope` whose lifetime spans the async boundary (see field note);
 // `EntryData.entry` likewise borrows.
-#[derive(Clone)]
+#[derive(Clone, Copy)]
 pub enum RefDataValue {
     Start,
     Collection {

src/runtime/test_runner/jest.rs

@@ -558,7 +558,7 @@ pub(crate) fn js_node_test_mark_result(
     // threaded JS VM, GC roots `done` (and its bound-this) for this frame.
     let (dcb_ref, dcb_called) = unsafe { ((*dcb).r#ref.as_deref(), (*dcb).called) };
     let bound = match dcb_ref {
-        Some(refdata) => refdata.phase.clone(),
+        Some(refdata) => refdata.phase,
         // `r#ref` unset: `.then()` fired inside run_test_callback's microtask
         // drain before it stamps the DoneCallback. `get_current_state_data()`
         // can't name a sequence inside a concurrent group, but

src/runtime/valkey_jsc/ValkeyCommand.rs

@@ -1,4 +1,3 @@
-use bun_collections::linear_fifo::{DynamicBuffer, LinearFifo};
 use bun_jsc::{self as jsc, JSGlobalObject, JSValue, JsResult};
 use bun_valkey::valkey_protocol as protocol;
 
@@ -114,7 +113,7 @@ pub struct Entry {
 // Inherent associated
 // types are unstable on stable Rust, so expose as a sibling module alias instead.
 pub mod entry {
-    pub(crate) type Queue = super::LinearFifo<super::Entry, super::DynamicBuffer<super::Entry>>;
+    pub(crate) type Queue = std::collections::VecDeque<super::Entry>;
 }
 
 impl Entry {
@@ -248,8 +247,7 @@ pub struct PromisePair {
 
 // See `entry` note above.
 pub mod promise_pair {
-    pub(crate) type Queue =
-        super::LinearFifo<super::PromisePair, super::DynamicBuffer<super::PromisePair>>;
+    pub(crate) type Queue = std::collections::VecDeque<super::PromisePair>;
 }
 
 impl PromisePair {

src/runtime/valkey_jsc/js_valkey.rs

@@ -758,8 +758,8 @@ impl JSValkeyClient {
                 protocol: uri,
                 username,
                 password,
-                in_flight: command::promise_pair::Queue::init(),
-                queue: command::entry::Queue::init(),
+                in_flight: command::promise_pair::Queue::new(),
+                queue: command::entry::Queue::new(),
                 status: valkey::Status::NeverConnected,
                 connection_strings,
                 socket: Socket::SocketTcp(uws::SocketTCP {
@@ -867,8 +867,8 @@ impl JSValkeyClient {
                 protocol: client.protocol,
                 username,
                 password,
-                in_flight: command::promise_pair::Queue::init(),
-                queue: command::entry::Queue::init(),
+                in_flight: command::promise_pair::Queue::new(),
+                queue: command::entry::Queue::new(),
                 status: valkey::Status::NeverConnected,
                 connection_strings: connection_strings_copy,
                 socket: Socket::SocketTcp(uws::SocketTCP {
@@ -1610,13 +1610,12 @@ impl JSValkeyClient {
         memory_cost += client.read_buffer.byte_list.capacity() as usize;
 
         // Add queue sizes
-        memory_cost += client.in_flight.readable_length()
-            * core::mem::size_of::<super::valkey_command::PromisePair>();
-        for command in client.queue.readable_slice(0) {
+        memory_cost +=
+            client.in_flight.len() * core::mem::size_of::<super::valkey_command::PromisePair>();
+        for command in client.queue.iter() {
             memory_cost += command.serialized_data.len();
         }
-        memory_cost +=
-            client.queue.readable_length() * core::mem::size_of::<super::valkey_command::Entry>();
+        memory_cost += client.queue.len() * core::mem::size_of::<super::valkey_command::Entry>();
         memory_cost
     }