diff --git a/crossbeam-skiplist/src/base.rs b/crossbeam-skiplist/src/base.rs index afa2df7a4..24ec88c4b 100644 --- a/crossbeam-skiplist/src/base.rs +++ b/crossbeam-skiplist/src/base.rs @@ -4,6 +4,7 @@ use alloc::alloc::handle_alloc_error; use core::{ alloc::Layout, cmp, fmt, + iter::FusedIterator, marker::PhantomData, mem, ops::{Bound, Deref, RangeBounds}, @@ -684,6 +685,7 @@ where range, guard, _marker: PhantomData, + finished: false, } } @@ -701,6 +703,7 @@ where head: None, tail: None, _marker: PhantomData, + finished: false, } } @@ -1988,6 +1991,7 @@ where range: R, guard: &'g Guard, _marker: PhantomData Q>, // covariant over `Q` + finished: bool, } impl<'a: 'g, 'g, Q, R, K: 'a, V: 'a, C> Iterator for Range<'a, 'g, Q, R, K, V, C> @@ -1999,6 +2003,9 @@ where type Item = Entry<'a, 'g, K, V, C>; fn next(&mut self) -> Option { + if self.finished { + return None; + } self.head = match self.head { Some(n) => self .parent @@ -2014,6 +2021,7 @@ where if !below_upper_bound(&self.parent.comparator, &bound, &h.key) { self.head = None; self.tail = None; + self.finished = true; } } None => { @@ -2021,6 +2029,7 @@ where if !below_upper_bound(&self.parent.comparator, &bound, &h.key) { self.head = None; self.tail = None; + self.finished = true; } } }; @@ -2040,6 +2049,9 @@ where Q: ?Sized, { fn next_back(&mut self) -> Option { + if self.finished { + return None; + } self.tail = match self.tail { Some(n) => self .parent @@ -2055,6 +2067,7 @@ where if !above_lower_bound(&self.parent.comparator, &bound, &t.key) { self.head = None; self.tail = None; + self.finished = true; } } None => { @@ -2062,6 +2075,7 @@ where if !above_lower_bound(&self.parent.comparator, &bound, &t.key) { self.head = None; self.tail = None; + self.finished = true; } } }; @@ -2074,6 +2088,14 @@ where } } +impl FusedIterator for Range<'_, '_, Q, R, K, V, C> +where + C: Comparator + Comparator, + R: RangeBounds, + Q: ?Sized, +{ +} + impl fmt::Debug for Range<'_, '_, Q, R, K, V, C> where C: Comparator + Comparator, @@ -2103,6 +2125,7 @@ where pub(crate) tail: Option>, pub(crate) range: R, _marker: PhantomData Q>, // covariant over `Q` + finished: bool, } unsafe impl Send for RefRange<'_, Q, R, K, V, C> @@ -2146,6 +2169,9 @@ where { /// Advances the iterator and returns the next value. pub fn next(&mut self, guard: &Guard) -> Option> { + if self.finished { + return None; + } self.parent.check_guard(guard); let next_head = match self.head { Some(ref e) => e.next(guard), @@ -2167,6 +2193,7 @@ where unsafe { h.node.decrement(guard); } + self.finished = true; None } } @@ -2183,6 +2210,7 @@ where unsafe { h.node.decrement(guard); } + self.finished = true; None } } @@ -2194,6 +2222,9 @@ where /// Removes and returns an element from the end of the iterator. pub fn next_back(&mut self, guard: &Guard) -> Option> { + if self.finished { + return None; + } self.parent.check_guard(guard); let next_tail = match self.tail { Some(ref e) => e.prev(guard), @@ -2215,6 +2246,7 @@ where unsafe { t.node.decrement(guard); } + self.finished = true; None } } @@ -2231,6 +2263,7 @@ where unsafe { t.node.decrement(guard); } + self.finished = true; None } } diff --git a/crossbeam-skiplist/src/map.rs b/crossbeam-skiplist/src/map.rs index 92e888ef4..5cf599e28 100644 --- a/crossbeam-skiplist/src/map.rs +++ b/crossbeam-skiplist/src/map.rs @@ -2,6 +2,7 @@ use core::{ fmt, + iter::FusedIterator, mem::ManuallyDrop, ops::{Bound, RangeBounds}, ptr, @@ -789,6 +790,14 @@ where } } +impl FusedIterator for Range<'_, Q, R, K, V, C> +where + C: Comparator + Comparator, + R: RangeBounds, + Q: ?Sized, +{ +} + impl fmt::Debug for Range<'_, Q, R, K, V, C> where C: Comparator + Comparator, diff --git a/crossbeam-skiplist/src/set.rs b/crossbeam-skiplist/src/set.rs index 7c931242e..fd7a340c1 100644 --- a/crossbeam-skiplist/src/set.rs +++ b/crossbeam-skiplist/src/set.rs @@ -2,6 +2,7 @@ use core::{ fmt, + iter::FusedIterator, ops::{Bound, Deref, RangeBounds}, }; @@ -644,6 +645,14 @@ where } } +impl FusedIterator for Range<'_, Q, R, T, C> +where + C: Comparator + Comparator, + R: RangeBounds, + Q: ?Sized, +{ +} + impl fmt::Debug for Range<'_, Q, R, T, C> where C: Comparator + Comparator, diff --git a/crossbeam-skiplist/tests/base.rs b/crossbeam-skiplist/tests/base.rs index c656adf1d..918594a0e 100644 --- a/crossbeam-skiplist/tests/base.rs +++ b/crossbeam-skiplist/tests/base.rs @@ -999,7 +999,9 @@ fn remove_race() { let mut removed_entries = Vec::with_capacity(KEY_RANGE as usize); barrier1.fetch_sub(1, Ordering::Relaxed); - while barrier1.load(Ordering::Acquire) != 0 {} + while barrier1.load(Ordering::Acquire) != 0 { + std::hint::spin_loop(); + } for x in 0..KEY_RANGE { if let Some(entry) = s.remove(&x, guard) { @@ -1008,7 +1010,9 @@ fn remove_race() { } barrier2.fetch_sub(1, Ordering::Relaxed); - while barrier2.load(Ordering::Acquire) != 0 {} + while barrier2.load(Ordering::Acquire) != 0 { + std::hint::spin_loop(); + } total_removed.fetch_add(removed_entries.len() as u32, Ordering::Relaxed); @@ -1021,3 +1025,23 @@ fn remove_race() { assert_eq!(*total_removed.get_mut(), KEY_RANGE); } + +// Regression test for https://github.com/crossbeam-rs/crossbeam/issues/1142 +#[test] +fn range_does_not_rewind_after_exhaustion() { + let guard = &epoch::pin(); + let s = SkipList::new(epoch::default_collector().clone()); + let v = (0..10).map(|x| x * 10).collect::>(); + for &x in v.iter() { + s.insert(x, x, guard).release(guard); + } + + // Range ..=5 should only yield the entry with key 0. + let mut it = s.range(..=5, guard); + assert_eq!(*it.next().unwrap().key(), 0); + assert!(it.next().is_none()); + + // After exhaustion, the iterator must not rewind to the beginning. + assert!(it.next().is_none()); + assert!(it.next().is_none()); +}