@@ -197,23 +197,24 @@ impl<T> TypedArena<T> {
197197 start_ptr
198198 }
199199
200+ /// Allocates the elements of this iterator into a contiguous slice in the `TypedArena`.
201+ ///
202+ /// Note: for reasons of reentrancy and panic safety we collect into a `SmallVec<[_; 8]>` before
203+ /// storing the elements in the arena.
200204#[ inline]
201205 pub fn alloc_from_iter < I : IntoIterator < Item = T > > ( & self , iter : I ) -> & mut [ T ] {
202- // This implementation is entirely separate to
203- // `DroplessIterator::alloc_from_iter`, even though conceptually they
204- // are the same .
206+ // Despite the similarlty with `DroplessArena`, we cannot reuse their fast case. The reason
207+ // is subtle: these arenas are reentrant. In other words, `iter` may very well be holding a
208+ // reference to `self` and adding elements to the arena during iteration .
205209 //
206- // `DroplessIterator` (in the fast case) writes elements from the
207- // iterator one at a time into the allocated memory. That's easy
208- // because the elements don't implement `Drop`. But for `TypedArena`
209- // they do implement `Drop`, which means that if the iterator panics we
210- // could end up with some allocated-but-uninitialized elements, which
211- // will then cause UB in `TypedArena::drop`.
210+ // For this reason, if we pre-allocated any space for the elements of this iterator, we'd
211+ // have to track that some uninitialized elements are followed by some initialized elements,
212+ // else we might accidentally drop uninitialized memory if something panics or if the
213+ // iterator doesn't fill all the length we expected.
212214 //
213- // Instead we use an approach where any iterator panic will occur
214- // before the memory is allocated. This function is much less hot than
215- // `DroplessArena::alloc_from_iter`, so it doesn't need to be
216- // hyper-optimized.
215+ // So we collect all the elements beforehand, which takes care of reentrancy and panic
216+ // safety. This function is much less hot than `DroplessArena::alloc_from_iter`, so it
217+ // doesn't need to be hyper-optimized.
217218 assert ! ( mem:: size_of:: <T >( ) != 0 ) ;
218219
219220 let mut vec: SmallVec < [ _ ; 8 ] > = iter. into_iter ( ) . collect ( ) ;
@@ -485,8 +486,9 @@ impl DroplessArena {
485486
486487 /// # Safety
487488///
488- /// The caller must ensure that `mem` is valid for writes up to
489- /// `size_of::<T>() * len`.
489+ /// The caller must ensure that `mem` is valid for writes up to `size_of::<T>() * len`, and that
490+ /// that memory stays allocated and not shared for the lifetime of `self`. This must hold even
491+ /// if `iter.next()` allocates onto `self`.
490492#[ inline]
491493 unsafe fn write_from_iter < T , I : Iterator < Item = T > > (
492494 & self ,
@@ -516,6 +518,8 @@ impl DroplessArena {
516518
517519 #[ inline]
518520 pub fn alloc_from_iter < T , I : IntoIterator < Item = T > > ( & self , iter : I ) -> & mut [ T ] {
521+ // Warning: this function is reentrant: `iter` could hold a reference to `&self` and
522+ // allocate additional elements while we're iterating.
519523 let iter = iter. into_iter ( ) ;
520524 assert ! ( mem:: size_of:: <T >( ) != 0 ) ;
521525 assert ! ( !mem:: needs_drop:: <T >( ) ) ;
@@ -524,18 +528,23 @@ impl DroplessArena {
524528
525529 match size_hint {
526530 ( min, Some ( max) ) if min == max => {
527- // We know the exact number of elements the iterator will produce here
531+ // We know the exact number of elements the iterator expects to produce here.
528532 let len = min;
529533
530534 if len == 0 {
531535 return & mut [ ] ;
532536 }
533537
534538 let mem = self . alloc_raw ( Layout :: array :: < T > ( len) . unwrap ( ) ) as * mut T ;
539+ // SAFETY: `write_from_iter` doesn't touch `self`. It only touches the slice we just
540+ // reserved. If the iterator panics or doesn't output `len` elements, this will
541+ // leave some unallocated slots in the arena, which is fine because we do not call
542+ // `drop`.
535543 unsafe { self . write_from_iter ( iter, len, mem) }
536544 }
537545 ( _, _) => {
538546 outline ( move || -> & mut [ T ] {
547+ // Takes care of reentrancy.
539548 let mut vec: SmallVec < [ _ ; 8 ] > = iter. collect ( ) ;
540549 if vec. is_empty ( ) {
541550 return & mut [ ] ;
0 commit comments