diff --git a/crates/oxc_allocator/src/vec2/mod.rs b/crates/oxc_allocator/src/vec2/mod.rs index d1b48f77d0126..21644df07f92a 100644 --- a/crates/oxc_allocator/src/vec2/mod.rs +++ b/crates/oxc_allocator/src/vec2/mod.rs @@ -720,26 +720,6 @@ impl<'a, T: 'a, A: Alloc> Vec<'a, T, A> { self.buf.set_len(new_len); } - /// Returns a shared reference to the allocator backing this `Vec`. - /// - /// # Examples - /// - /// ``` - /// use bumpalo::{Bump, collections::Vec}; - /// - /// // uses the same allocator as the provided `Vec` - /// fn add_strings<'a>(vec: &mut Vec<'a, &'a str>) { - /// for string in ["foo", "bar", "baz"] { - /// vec.push(vec.bump().alloc_str(string)); - /// } - /// } - /// ``` - #[inline] - #[must_use] - pub fn bump(&self) -> &'a A { - self.buf.bump() - } - /// Reserves capacity for at least `additional` more elements to be inserted /// in the given `Vec<'a, T>`. The collection may reserve more space to avoid /// frequent reallocations. After calling `reserve`, capacity will be @@ -1735,7 +1715,10 @@ impl<'a, T: 'a, A: Alloc> Vec<'a, T, A> { assert!(at <= self.len_usize(), "`at` out of bounds"); let other_len = self.len_usize() - at; - let mut other = Vec::with_capacity_in(other_len, self.buf.bump()); + // SAFETY: This method takes a `&mut self`. It lives for the duration of this method + // - longer than we use `bump` for. + let bump = unsafe { self.buf.bump() }; + let mut other = Vec::with_capacity_in(other_len, bump); // Unsafely `set_len` and copy items to `other`. unsafe { @@ -2683,7 +2666,15 @@ impl Drop for Splice<'_, '_, I, A> { // Collect any remaining elements. // This is a zero-length vector which does not allocate if `lower_bound` was exact. - let mut collected = Vec::new_in(self.drain.vec.as_ref().buf.bump()); + + // SAFETY: `Splice` iterator is created in `Vec::splice`, which takes a `&mut self`. + // `Splice` inherits the lifetime of `&mut self` from that method, so the mut borrow + // of the `Vec` is held for the life of the `Splice`. + // Therefore we have exclusive access to the `Vec` until end of this method. + // That is longer than we use `bump` for. + let bump = self.drain.vec.as_ref().buf.bump(); + + let mut collected = Vec::new_in(bump); collected.extend(self.replace_with.by_ref()); let mut collected = collected.into_iter(); // Now we have an exact count. diff --git a/crates/oxc_allocator/src/vec2/raw_vec.rs b/crates/oxc_allocator/src/vec2/raw_vec.rs index dc558427c4b83..c4572d8e62ee2 100644 --- a/crates/oxc_allocator/src/vec2/raw_vec.rs +++ b/crates/oxc_allocator/src/vec2/raw_vec.rs @@ -72,6 +72,8 @@ pub struct RawVec<'a, T, A: Alloc> { ptr: NonNull, len: u32, cap: u32, + // SAFETY: Methods must not mutate the allocator (e.g. allocate into it), unless they can guarantee + // they have exclusive access to it, by taking `&mut self` alloc: &'a A, } @@ -192,8 +194,47 @@ impl<'a, T, A: Alloc> RawVec<'a, T, A> { if mem::size_of::() == 0 { !0 } else { self.cap as usize } } - /// Returns a shared reference to the allocator backing this RawVec. - pub fn bump(&self) -> &'a A { + /// Get a shared reference to the allocator backing this `RawVec`. + /// + /// This method is hazardous. + /// + /// `Vec` is `Sync`, but `Bump` is not, because it utilizes interior mutability. + /// It is possible to make allocations into the arena while holding only a `&Bump`. + /// Because `Vec` is `Sync`, it's possible for multiple `&Vec` references to the same `Vec`, + /// or references to multiple `Vec`s attached to the same `Bump`, to exist simultaneously + /// on different threads. + /// + /// So this method could be used to obtain 2 `&Bump` references simultaneously on different threads. + /// Utilizing those references to allocate into the arena simultaneously from different threads + /// would be UB. + /// + /// We cannot rely on the type system or borrow checker to ensure correct synchronization. + /// + /// Therefore callers must ensure by other means that they have exclusive access, by: + /// + /// 1. Taking a `&mut self`. + /// No methods of `Vec` or `RawVec` which do not hold a `&mut Vec` / `&mut RawVec` can use this method. + /// + /// 2. That `&mut self` must be held for at least as long as the `&'a A` reference returned by + /// this method is held. + /// + /// Note: It's tempting to think we could make this a safe method by making it take `&mut self`, + /// but that's insufficient. That would enforce that the caller holds a `&mut self`, but the `&'a A` + /// returned by this method outlives the lifetime of `&self` that the method takes, so it would + /// NOT guarantee anything about *how long* they hold it for. + /// Taking a `&'a mut self` *would* be safe, but it'd be impractical. + /// + /// For further information, see comments on the `impl Sync` implementation of `Vec`. + /// + /// # IMPORTANT + /// The ability to obtain a reference to the allocator MUST NOT be exposed to user, + /// outside of `Vec`'s internals. + /// + /// # SAFETY + /// Caller must ensure they have exclusive access, but holding a `&mut Vec` or `&mut RawVec` + /// for the duration that the reference returned by this method is held. + /// See text above for further detail. + pub unsafe fn bump(&self) -> &'a A { self.alloc }