From 47503e000adf84eb7890e7eb21306697d9a25bbc Mon Sep 17 00:00:00 2001 From: Jeonghyeon Kim <49356944+powergee@users.noreply.github.com> Date: Tue, 16 Jul 2024 23:42:23 +0900 Subject: [PATCH 01/18] Fix all warnings from the latest Clippy (#1123) --- Cargo.toml | 4 ++++ crossbeam-channel/src/select_macro.rs | 2 +- crossbeam-skiplist/src/lib.rs | 6 +++--- crossbeam-utils/src/atomic/atomic_cell.rs | 1 - 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 14f46133e..0daf834ea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -71,3 +71,7 @@ unexpected_cfgs = { level = "warn", check-cfg = [ 'cfg(crossbeam_loom)', 'cfg(crossbeam_sanitize)', ] } + +[workspace.lints.clippy] +# Suppress buggy or noisy clippy lints +declare_interior_mutable_const = { level = "allow", priority = 1 } # https://github.com/rust-lang/rust-clippy/issues/7665 diff --git a/crossbeam-channel/src/select_macro.rs b/crossbeam-channel/src/select_macro.rs index e109404f7..154e2540c 100644 --- a/crossbeam-channel/src/select_macro.rs +++ b/crossbeam-channel/src/select_macro.rs @@ -687,7 +687,7 @@ macro_rules! crossbeam_channel_internal { const _LEN: usize = $crate::crossbeam_channel_internal!(@count ($($cases)*)); let _handle: &dyn $crate::internal::SelectHandle = &$crate::never::<()>(); - #[allow(unused_mut)] + #[allow(unused_mut, clippy::zero_repeat_side_effects)] let mut _sel = [(_handle, 0, ::std::ptr::null()); _LEN]; $crate::crossbeam_channel_internal!( diff --git a/crossbeam-skiplist/src/lib.rs b/crossbeam-skiplist/src/lib.rs index 38628d5ae..30552faf7 100644 --- a/crossbeam-skiplist/src/lib.rs +++ b/crossbeam-skiplist/src/lib.rs @@ -91,9 +91,9 @@ //! A solution to the above is to have the implementation wrap //! each value in a lock. However, this has some repercussions: //! * The map would no longer be lock-free, inhibiting scalability -//! and allowing for deadlocks. +//! and allowing for deadlocks. //! * If a user of the map doesn't need mutable access, then they pay -//! the price of locks without actually needing them. +//! the price of locks without actually needing them. //! //! Instead, the approach taken by this crate gives more control to the user. //! If mutable access is needed, then you can use interior mutability, @@ -150,7 +150,7 @@ //! Crossbeam [does not currently provide a concurrent unordered map](https://github.com/crossbeam-rs/rfcs/issues/32). //! That said, here are some other crates which may suit you: //! * [`DashMap`](https://docs.rs/dashmap) implements a novel concurrent hash map -//! with good performance characteristics. +//! with good performance characteristics. //! * [`flurry`](https://docs.rs/flurry) is a Rust port of Java's `ConcurrentHashMap`. //! //! [`insert`]: SkipMap::insert diff --git a/crossbeam-utils/src/atomic/atomic_cell.rs b/crossbeam-utils/src/atomic/atomic_cell.rs index 06ccf2eb5..a603c1d36 100644 --- a/crossbeam-utils/src/atomic/atomic_cell.rs +++ b/crossbeam-utils/src/atomic/atomic_cell.rs @@ -1000,7 +1000,6 @@ fn lock(addr: usize) -> &'static SeqLock { // stored at addresses that are multiples of 3. It'd be too bad if `LEN` was divisible by 3. // In order to protect from such cases, we simply choose a large prime number for `LEN`. const LEN: usize = 67; - #[allow(clippy::declare_interior_mutable_const)] const L: CachePadded = CachePadded::new(SeqLock::new()); static LOCKS: [CachePadded; LEN] = [L; LEN]; From 4966fa40a3025bbb2ea509e52d9a1d880ab21f42 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sat, 22 Jun 2024 03:40:09 +0900 Subject: [PATCH 02/18] deque: Use ptr::slice_from_raw_parts_mut --- crossbeam-deque/src/deque.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crossbeam-deque/src/deque.rs b/crossbeam-deque/src/deque.rs index 7961828ad..b7ecf9889 100644 --- a/crossbeam-deque/src/deque.rs +++ b/crossbeam-deque/src/deque.rs @@ -5,7 +5,6 @@ use std::fmt; use std::marker::PhantomData; use std::mem::{self, MaybeUninit}; use std::ptr; -use std::slice; use std::sync::atomic::{self, AtomicIsize, AtomicPtr, AtomicUsize, Ordering}; use std::sync::Arc; @@ -51,7 +50,7 @@ impl Buffer { /// Deallocates the buffer. unsafe fn dealloc(self) { - drop(Box::from_raw(slice::from_raw_parts_mut( + drop(Box::from_raw(ptr::slice_from_raw_parts_mut( self.ptr.cast::>(), self.cap, ))); From a85b3aa5e25594cf5defdc8b23101e67de432177 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Thu, 1 Aug 2024 02:06:34 +0900 Subject: [PATCH 03/18] Disable buggy clippy::lint_groups_priority lint ``` error: lint group `rust_2018_idioms` has the same priority (0) as a lint --> Cargo.toml:64:1 | 64 | rust_2018_idioms = "warn" | ^^^^^^^^^^^^^^^^ ------ has an implicit priority of 0 65 | single_use_lifetimes = "warn" | -------------------- has the same priority as this lint | = note: the order of the lints in the table is ignored by Cargo = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#lint_groups_priority = note: `#[deny(clippy::lint_groups_priority)]` on by default help: to have lints override the group set `rust_2018_idioms` to a lower priority | 64 | rust_2018_idioms = { level = "warn", priority = -1 } | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 0daf834ea..3e8b4a3c5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -71,7 +71,7 @@ unexpected_cfgs = { level = "warn", check-cfg = [ 'cfg(crossbeam_loom)', 'cfg(crossbeam_sanitize)', ] } - [workspace.lints.clippy] # Suppress buggy or noisy clippy lints declare_interior_mutable_const = { level = "allow", priority = 1 } # https://github.com/rust-lang/rust-clippy/issues/7665 +lint_groups_priority = { level = "allow", priority = 1 } # https://github.com/rust-lang/rust-clippy/issues/12920 From bd85a146b873def323d35b2d1c18ae1eed6596ff Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Thu, 1 Aug 2024 02:07:41 +0900 Subject: [PATCH 04/18] Temporarily disable flaky fairness2 test with sanitizers https://github.com/crossbeam-rs/crossbeam/issues/1094 --- crossbeam-channel/tests/select_macro.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crossbeam-channel/tests/select_macro.rs b/crossbeam-channel/tests/select_macro.rs index 794e026b8..aaffa704e 100644 --- a/crossbeam-channel/tests/select_macro.rs +++ b/crossbeam-channel/tests/select_macro.rs @@ -858,6 +858,7 @@ fn fairness1() { assert!(hits.iter().all(|x| *x >= COUNT / hits.len() / 2)); } +#[cfg_attr(crossbeam_sanitize, ignore)] // TODO: flaky: https://github.com/crossbeam-rs/crossbeam/issues/1094 #[test] fn fairness2() { #[cfg(miri)] From fd3949294511015fea4234582893cc48190353be Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 7 Nov 2024 17:53:05 -0800 Subject: [PATCH 05/18] channel: Create `list::Block` directly on the heap (#1146) The list channel's `Block::new` was causing a stack overflow because it held 32 item slots, instantiated on the stack before moving to `Box::new`. The 32x multiplier made modestly-large item sizes untenable. That block is now initialized directly on the heap. References from the `std` channel implementation: * https://github.com/rust-lang/rust/issues/102246 * https://github.com/rust-lang/rust/pull/132738 --- crossbeam-channel/src/flavors/list.rs | 25 +++++++++++++------------ crossbeam-channel/tests/list.rs | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/crossbeam-channel/src/flavors/list.rs b/crossbeam-channel/src/flavors/list.rs index e7fb6150f..1b486d220 100644 --- a/crossbeam-channel/src/flavors/list.rs +++ b/crossbeam-channel/src/flavors/list.rs @@ -1,5 +1,6 @@ //! Unbounded channel implemented as a linked list. +use std::alloc::{alloc_zeroed, Layout}; use std::boxed::Box; use std::cell::UnsafeCell; use std::marker::PhantomData; @@ -50,11 +51,6 @@ struct Slot { } impl Slot { - const UNINIT: Self = Self { - msg: UnsafeCell::new(MaybeUninit::uninit()), - state: AtomicUsize::new(0), - }; - /// Waits until a message is written into the slot. fn wait_write(&self) { let backoff = Backoff::new(); @@ -77,11 +73,16 @@ struct Block { impl Block { /// Creates an empty block. - fn new() -> Block { - Self { - next: AtomicPtr::new(ptr::null_mut()), - slots: [Slot::UNINIT; BLOCK_CAP], - } + fn new() -> Box { + // SAFETY: This is safe because: + // [1] `Block::next` (AtomicPtr) may be safely zero initialized. + // [2] `Block::slots` (Array) may be safely zero initialized because of [3, 4]. + // [3] `Slot::msg` (UnsafeCell) may be safely zero initialized because it + // holds a MaybeUninit. + // [4] `Slot::state` (AtomicUsize) may be safely zero initialized. + // TODO: unsafe { Box::new_zeroed().assume_init() } + let layout = Layout::new::(); + unsafe { Box::from_raw(alloc_zeroed(layout).cast()) } } /// Waits until the next pointer is set. @@ -223,13 +224,13 @@ impl Channel { // If we're going to have to install the next block, allocate it in advance in order to // make the wait for other threads as short as possible. if offset + 1 == BLOCK_CAP && next_block.is_none() { - next_block = Some(Box::new(Block::::new())); + next_block = Some(Block::::new()); } // If this is the first message to be sent into the channel, we need to allocate the // first block and install it. if block.is_null() { - let new = Box::into_raw(Box::new(Block::::new())); + let new = Box::into_raw(Block::::new()); if self .tail diff --git a/crossbeam-channel/tests/list.rs b/crossbeam-channel/tests/list.rs index ebe6f6f85..beabac8f2 100644 --- a/crossbeam-channel/tests/list.rs +++ b/crossbeam-channel/tests/list.rs @@ -580,3 +580,18 @@ fn channel_through_channel() { }) .unwrap(); } + +// If `Block` is created on the stack, the array of slots will multiply this `BigStruct` and +// probably overflow the thread stack. It's now directly created on the heap to avoid this. +#[test] +fn stack_overflow() { + const N: usize = 32_768; + struct BigStruct { + _data: [u8; N], + } + + let (sender, receiver) = unbounded::(); + sender.send(BigStruct { _data: [0u8; N] }).unwrap(); + + for _data in receiver.try_iter() {} +} From 01fff03a63dae4814c182708fabbe7b639311447 Mon Sep 17 00:00:00 2001 From: zachs18 <8355914+zachs18@users.noreply.github.com> Date: Fri, 8 Nov 2024 10:48:45 +0000 Subject: [PATCH 06/18] channel: Handle possible allocation failure in `list::Block::new` (#1147) --- crossbeam-channel/src/flavors/list.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/crossbeam-channel/src/flavors/list.rs b/crossbeam-channel/src/flavors/list.rs index 1b486d220..9f2e55258 100644 --- a/crossbeam-channel/src/flavors/list.rs +++ b/crossbeam-channel/src/flavors/list.rs @@ -1,6 +1,6 @@ //! Unbounded channel implemented as a linked list. -use std::alloc::{alloc_zeroed, Layout}; +use std::alloc::{alloc_zeroed, handle_alloc_error, Layout}; use std::boxed::Box; use std::cell::UnsafeCell; use std::marker::PhantomData; @@ -74,6 +74,17 @@ struct Block { impl Block { /// Creates an empty block. fn new() -> Box { + let layout = Layout::new::(); + assert!( + layout.size() != 0, + "Block should never be zero-sized, as it has an AtomicPtr field" + ); + // SAFETY: layout is not zero-sized + let ptr = unsafe { alloc_zeroed(layout) }; + // Handle allocation failure + if ptr.is_null() { + handle_alloc_error(layout) + } // SAFETY: This is safe because: // [1] `Block::next` (AtomicPtr) may be safely zero initialized. // [2] `Block::slots` (Array) may be safely zero initialized because of [3, 4]. @@ -81,8 +92,7 @@ impl Block { // holds a MaybeUninit. // [4] `Slot::state` (AtomicUsize) may be safely zero initialized. // TODO: unsafe { Box::new_zeroed().assume_init() } - let layout = Layout::new::(); - unsafe { Box::from_raw(alloc_zeroed(layout).cast()) } + unsafe { Box::from_raw(ptr.cast()) } } /// Waits until the next pointer is set. From 3bc6edcee12a9db8c4da79af624b9bdcbc5bff2e Mon Sep 17 00:00:00 2001 From: tison Date: Fri, 8 Nov 2024 18:50:00 +0800 Subject: [PATCH 07/18] Clearly document SegQueue is FIFO (#1144) Signed-off-by: tison --- crossbeam-queue/src/seg_queue.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crossbeam-queue/src/seg_queue.rs b/crossbeam-queue/src/seg_queue.rs index 973a77f81..620a8f6aa 100644 --- a/crossbeam-queue/src/seg_queue.rs +++ b/crossbeam-queue/src/seg_queue.rs @@ -176,7 +176,7 @@ impl SegQueue { } } - /// Pushes an element into the queue. + /// Pushes back an element to the tail. /// /// # Examples /// @@ -268,7 +268,7 @@ impl SegQueue { } } - /// Pops an element from the queue. + /// Pops the head element from the queue. /// /// If the queue is empty, `None` is returned. /// @@ -280,7 +280,9 @@ impl SegQueue { /// let q = SegQueue::new(); /// /// q.push(10); + /// q.push(20); /// assert_eq!(q.pop(), Some(10)); + /// assert_eq!(q.pop(), Some(20)); /// assert!(q.pop().is_none()); /// ``` pub fn pop(&self) -> Option { From 2ae95ba8d92054551b4fce1ed24fea2488e60e9b Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 8 Dec 2024 17:05:40 +0900 Subject: [PATCH 08/18] ci: Use GITHUB_OUTPUT instead of deprecated set-output --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7e339daf7..e9a00913b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -136,7 +136,7 @@ jobs: if ! git diff --exit-code; then git add . git commit -m "Update no_atomic.rs" - echo "::set-output name=success::false" + echo 'success=false' >>"${GITHUB_OUTPUT}" fi if: github.repository_owner == 'crossbeam-rs' && github.event_name == 'schedule' - uses: peter-evans/create-pull-request@v5 From 2253c5b5102c93da5d92df2818e65b2db89111a4 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 8 Dec 2024 17:15:10 +0900 Subject: [PATCH 09/18] ci: Clean up no_atomic.sh --- ci/no_atomic.sh | 47 +++++++++++++---------------------------------- 1 file changed, 13 insertions(+), 34 deletions(-) diff --git a/ci/no_atomic.sh b/ci/no_atomic.sh index 288150b3b..13ab417f3 100755 --- a/ci/no_atomic.sh +++ b/ci/no_atomic.sh @@ -8,45 +8,24 @@ cd "$(dirname "$0")"/.. # Usage: # ./ci/no_atomic.sh -file="no_atomic.rs" +file=no_atomic.rs -no_atomic=() -for target_spec in $(RUSTC_BOOTSTRAP=1 rustc +stable -Z unstable-options --print all-target-specs-json | jq -c '. | to_entries | .[]'); do - target=$(jq <<<"${target_spec}" -r '.key') - target_spec=$(jq <<<"${target_spec}" -c '.value') - res=$(jq <<<"${target_spec}" -r 'select(."atomic-cas" == false)') - [[ -z "${res}" ]] || no_atomic_cas+=("${target}") - max_atomic_width=$(jq <<<"${target_spec}" -r '."max-atomic-width"') - min_atomic_width=$(jq <<<"${target_spec}" -r '."min-atomic-width"') - case "${max_atomic_width}" in - # `"max-atomic-width" == 0` means that atomic is not supported at all. - # We do not have a cfg for targets with {8,16}-bit atomic only, so - # for now we treat them the same as targets that do not support atomic. - 0) no_atomic+=("${target}") ;; - # It is not clear exactly what `"max-atomic-width" == null` means, but they - # actually seem to have the same max-atomic-width as the target-pointer-width. - # The targets currently included in this group are "mipsel-sony-psp", - # "thumbv4t-none-eabi", "thumbv6m-none-eabi", all of which are - # `"target-pointer-width" == "32"`, so assuming them `"max-atomic-width" == 32` - # for now. - null | 8 | 16 | 32 | 64 | 128) ;; - *) exit 1 ;; - esac - case "${min_atomic_width}" in - 8 | null) ;; - *) no_atomic+=("${target}") ;; - esac -done +# `"max-atomic-width" == 0` means that atomic is not supported at all. +# We do not have a cfg for targets with {8,16}-bit atomic only, so +# for now we treat them the same as targets that do not support atomic. +# It is not clear exactly what `"max-atomic-width" == null` means, but they +# actually seem to have the same max-atomic-width as the target-pointer-width. +# The targets currently included in this group are "mipsel-sony-psp", +# "thumbv4t-none-eabi", "thumbv6m-none-eabi", all of which are +# `"target-pointer-width" == "32"`, so assuming them `"max-atomic-width" == 32` +# for now. +no_atomic=$(RUSTC_BOOTSTRAP=1 rustc +stable -Z unstable-options --print all-target-specs-json | jq -r '. | to_entries[] | select((.value."max-atomic-width" == 0) or (.value."min-atomic-width" and .value."min-atomic-width" != 8)) | " \"" + .key + "\","') cat >"${file}" <>"${file}" -done -cat >>"${file}" < Date: Sun, 8 Dec 2024 17:15:15 +0900 Subject: [PATCH 10/18] ci: Update minimum tested Rust version to 1.63 ``` error: package `libc v0.2.167` cannot be built because it requires rustc 1.63 or newer, while the currently active rustc version is 1.61.0 ``` --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e9a00913b..15e62c89d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,9 +35,9 @@ jobs: fail-fast: false matrix: include: - - rust: '1.61' + - rust: '1.63' os: ubuntu-latest - - rust: '1.61' + - rust: '1.63' os: windows-latest - rust: stable os: ubuntu-latest From 92999cadbbe394a930f12aac81db1d649c4d7e54 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 8 Dec 2024 17:15:19 +0900 Subject: [PATCH 11/18] Add comment about fixed rustc bug --- crossbeam-utils/src/atomic/atomic_cell.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crossbeam-utils/src/atomic/atomic_cell.rs b/crossbeam-utils/src/atomic/atomic_cell.rs index a603c1d36..47472534c 100644 --- a/crossbeam-utils/src/atomic/atomic_cell.rs +++ b/crossbeam-utils/src/atomic/atomic_cell.rs @@ -36,6 +36,7 @@ pub struct AtomicCell { /// /// Using MaybeUninit to prevent code outside the cell from observing partially initialized state: /// + /// (This rustc bug has been fixed in Rust 1.64.) /// /// Note: /// - we'll never store uninitialized `T` due to our API only using initialized `T`. From 72410f2e7e3e1bd98aeb5563a4e3e0b76be39856 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 8 Dec 2024 19:27:23 +0900 Subject: [PATCH 12/18] CachePadded: Use 128-byte alignment on arm64ec Same as aarch64. --- crossbeam-utils/src/cache_padded.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crossbeam-utils/src/cache_padded.rs b/crossbeam-utils/src/cache_padded.rs index 44212b835..6c930c6f3 100644 --- a/crossbeam-utils/src/cache_padded.rs +++ b/crossbeam-utils/src/cache_padded.rs @@ -67,7 +67,7 @@ use core::ops::{Deref, DerefMut}; // - https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-optimization-manual.pdf // - https://github.com/facebook/folly/blob/1b5288e6eea6df074758f877c849b6e73bbb9fbb/folly/lang/Align.h#L107 // -// ARM's big.LITTLE architecture has asymmetric cores and "big" cores have 128-byte cache line size. +// aarch64/arm64ec's big.LITTLE architecture has asymmetric cores and "big" cores have 128-byte cache line size. // // Sources: // - https://www.mono-project.com/news/2016/09/12/arm64-icache/ @@ -81,6 +81,7 @@ use core::ops::{Deref, DerefMut}; any( target_arch = "x86_64", target_arch = "aarch64", + target_arch = "arm64ec", target_arch = "powerpc64", ), repr(align(128)) @@ -130,6 +131,7 @@ use core::ops::{Deref, DerefMut}; not(any( target_arch = "x86_64", target_arch = "aarch64", + target_arch = "arm64ec", target_arch = "powerpc64", target_arch = "arm", target_arch = "mips", From 9ee87995220566d206ea701d38a54eb4bbfbce73 Mon Sep 17 00:00:00 2001 From: Jakob Kraus <52459467+JabobKrauskopf@users.noreply.github.com> Date: Sun, 8 Dec 2024 13:13:29 +0100 Subject: [PATCH 13/18] channel: Add new_biased constructor for biased channel selection (#1150) --- crossbeam-channel/src/select.rs | 40 +++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/crossbeam-channel/src/select.rs b/crossbeam-channel/src/select.rs index 4bfa65c7f..36e68b22d 100644 --- a/crossbeam-channel/src/select.rs +++ b/crossbeam-channel/src/select.rs @@ -611,6 +611,9 @@ pub struct Select<'a> { /// The next index to assign to an operation. next_index: usize, + + /// Whether to use the index of handles as bias for selecting ready operations. + biased: bool, } unsafe impl Send for Select<'_> {} @@ -633,6 +636,28 @@ impl<'a> Select<'a> { Select { handles: Vec::with_capacity(4), next_index: 0, + biased: false, + } + } + + /// Creates an empty list of channel operations with biased selection. + /// + /// When multiple handles are ready, this will select the operation with the lowest index. + /// + /// # Examples + /// + /// ``` + /// use crossbeam_channel::Select; + /// + /// let mut sel = Select::new_biased(); + /// + /// // The list of operations is empty, which means no operation can be selected. + /// assert!(sel.try_select().is_err()); + /// ``` + pub fn new_biased() -> Self { + Self { + biased: true, + ..Default::default() } } @@ -774,7 +799,7 @@ impl<'a> Select<'a> { /// } /// ``` pub fn try_select(&mut self) -> Result, TrySelectError> { - try_select(&mut self.handles, false) + try_select(&mut self.handles, self.biased) } /// Blocks until one of the operations becomes ready and selects it. @@ -821,7 +846,7 @@ impl<'a> Select<'a> { /// } /// ``` pub fn select(&mut self) -> SelectedOperation<'a> { - select(&mut self.handles, false) + select(&mut self.handles, self.biased) } /// Blocks for a limited time until one of the operations becomes ready and selects it. @@ -871,7 +896,7 @@ impl<'a> Select<'a> { &mut self, timeout: Duration, ) -> Result, SelectTimeoutError> { - select_timeout(&mut self.handles, timeout, false) + select_timeout(&mut self.handles, timeout, self.biased) } /// Blocks until a given deadline, or until one of the operations becomes ready and selects it. @@ -923,7 +948,7 @@ impl<'a> Select<'a> { &mut self, deadline: Instant, ) -> Result, SelectTimeoutError> { - select_deadline(&mut self.handles, deadline, false) + select_deadline(&mut self.handles, deadline, self.biased) } /// Attempts to find a ready operation without blocking. @@ -962,7 +987,7 @@ impl<'a> Select<'a> { /// } /// ``` pub fn try_ready(&mut self) -> Result { - match run_ready(&mut self.handles, Timeout::Now, false) { + match run_ready(&mut self.handles, Timeout::Now, self.biased) { None => Err(TryReadyError), Some(index) => Ok(index), } @@ -1015,7 +1040,7 @@ impl<'a> Select<'a> { panic!("no operations have been added to `Select`"); } - run_ready(&mut self.handles, Timeout::Never, false).unwrap() + run_ready(&mut self.handles, Timeout::Never, self.biased).unwrap() } /// Blocks for a limited time until one of the operations becomes ready. @@ -1108,7 +1133,7 @@ impl<'a> Select<'a> { /// } /// ``` pub fn ready_deadline(&mut self, deadline: Instant) -> Result { - match run_ready(&mut self.handles, Timeout::At(deadline), false) { + match run_ready(&mut self.handles, Timeout::At(deadline), self.biased) { None => Err(ReadyTimeoutError), Some(index) => Ok(index), } @@ -1120,6 +1145,7 @@ impl<'a> Clone for Select<'a> { Select { handles: self.handles.clone(), next_index: self.next_index, + biased: self.biased, } } } From 79b240ffc9aa6ab280b4932f8db2046f2a9fa4d4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 8 Dec 2024 21:14:10 +0900 Subject: [PATCH 14/18] Bump peter-evans/create-pull-request from 5 to 7 (#1153) Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 15e62c89d..454958df6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -139,7 +139,7 @@ jobs: echo 'success=false' >>"${GITHUB_OUTPUT}" fi if: github.repository_owner == 'crossbeam-rs' && github.event_name == 'schedule' - - uses: peter-evans/create-pull-request@v5 + - uses: peter-evans/create-pull-request@v7 with: title: Update no_atomic.rs body: | From 07c164f5e6d32607a2aa964eacdacc1b1555834d Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 8 Dec 2024 22:21:21 +0900 Subject: [PATCH 15/18] Remove optimistic spinning from Context::wait_until Co-authored-by: Ibraheem Ahmed --- crossbeam-channel/src/context.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/crossbeam-channel/src/context.rs b/crossbeam-channel/src/context.rs index 4230027f0..993a420a4 100644 --- a/crossbeam-channel/src/context.rs +++ b/crossbeam-channel/src/context.rs @@ -138,21 +138,6 @@ impl Context { /// If the deadline is reached, `Selected::Aborted` will be selected. #[inline] pub fn wait_until(&self, deadline: Option) -> Selected { - // Spin for a short time, waiting until an operation is selected. - let backoff = Backoff::new(); - loop { - let sel = Selected::from(self.inner.select.load(Ordering::Acquire)); - if sel != Selected::Waiting { - return sel; - } - - if backoff.is_completed() { - break; - } else { - backoff.snooze(); - } - } - loop { // Check whether an operation has been selected. let sel = Selected::from(self.inner.select.load(Ordering::Acquire)); From 72668e059dc78c380bb8628ea12fc6684865b4dc Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 15 Dec 2024 23:22:00 +0900 Subject: [PATCH 16/18] Port #1146 & #1147 to deque::Injector and queue::SegQueue --- crossbeam-deque/src/deque.rs | 35 ++++++++++++++++++++---------- crossbeam-deque/tests/injector.rs | 16 ++++++++++++++ crossbeam-queue/src/seg_queue.rs | 35 ++++++++++++++++++++---------- crossbeam-queue/tests/seg_queue.rs | 15 +++++++++++++ 4 files changed, 77 insertions(+), 24 deletions(-) diff --git a/crossbeam-deque/src/deque.rs b/crossbeam-deque/src/deque.rs index b7ecf9889..27354e09e 100644 --- a/crossbeam-deque/src/deque.rs +++ b/crossbeam-deque/src/deque.rs @@ -1,3 +1,4 @@ +use std::alloc::{alloc_zeroed, handle_alloc_error, Layout}; use std::boxed::Box; use std::cell::{Cell, UnsafeCell}; use std::cmp; @@ -1203,11 +1204,6 @@ struct Slot { } impl Slot { - const UNINIT: Self = Self { - task: UnsafeCell::new(MaybeUninit::uninit()), - state: AtomicUsize::new(0), - }; - /// Waits until a task is written into the slot. fn wait_write(&self) { let backoff = Backoff::new(); @@ -1229,12 +1225,27 @@ struct Block { } impl Block { - /// Creates an empty block that starts at `start_index`. - fn new() -> Block { - Self { - next: AtomicPtr::new(ptr::null_mut()), - slots: [Slot::UNINIT; BLOCK_CAP], + /// Creates an empty block. + fn new() -> Box { + let layout = Layout::new::(); + assert!( + layout.size() != 0, + "Block should never be zero-sized, as it has an AtomicPtr field" + ); + // SAFETY: layout is not zero-sized + let ptr = unsafe { alloc_zeroed(layout) }; + // Handle allocation failure + if ptr.is_null() { + handle_alloc_error(layout) } + // SAFETY: This is safe because: + // [1] `Block::next` (AtomicPtr) may be safely zero initialized. + // [2] `Block::slots` (Array) may be safely zero initialized because of [3, 4]. + // [3] `Slot::task` (UnsafeCell) may be safely zero initialized because it + // holds a MaybeUninit. + // [4] `Slot::state` (AtomicUsize) may be safely zero initialized. + // TODO: unsafe { Box::new_zeroed().assume_init() } + unsafe { Box::from_raw(ptr.cast()) } } /// Waits until the next pointer is set. @@ -1313,7 +1324,7 @@ unsafe impl Sync for Injector {} impl Default for Injector { fn default() -> Self { - let block = Box::into_raw(Box::new(Block::::new())); + let block = Box::into_raw(Block::::new()); Self { head: CachePadded::new(Position { block: AtomicPtr::new(block), @@ -1374,7 +1385,7 @@ impl Injector { // If we're going to have to install the next block, allocate it in advance in order to // make the wait for other threads as short as possible. if offset + 1 == BLOCK_CAP && next_block.is_none() { - next_block = Some(Box::new(Block::::new())); + next_block = Some(Block::::new()); } let new_tail = tail + (1 << SHIFT); diff --git a/crossbeam-deque/tests/injector.rs b/crossbeam-deque/tests/injector.rs index f706a8d9c..5f6c3e98e 100644 --- a/crossbeam-deque/tests/injector.rs +++ b/crossbeam-deque/tests/injector.rs @@ -373,3 +373,19 @@ fn destructors() { } } } + +// If `Block` is created on the stack, the array of slots will multiply this `BigStruct` and +// probably overflow the thread stack. It's now directly created on the heap to avoid this. +#[test] +fn stack_overflow() { + const N: usize = 32_768; + struct BigStruct { + _data: [u8; N], + } + + let q = Injector::new(); + + q.push(BigStruct { _data: [0u8; N] }); + + while !matches!(q.steal(), Empty) {} +} diff --git a/crossbeam-queue/src/seg_queue.rs b/crossbeam-queue/src/seg_queue.rs index 620a8f6aa..68197e896 100644 --- a/crossbeam-queue/src/seg_queue.rs +++ b/crossbeam-queue/src/seg_queue.rs @@ -1,3 +1,4 @@ +use alloc::alloc::{alloc_zeroed, handle_alloc_error, Layout}; use alloc::boxed::Box; use core::cell::UnsafeCell; use core::fmt; @@ -36,11 +37,6 @@ struct Slot { } impl Slot { - const UNINIT: Self = Self { - value: UnsafeCell::new(MaybeUninit::uninit()), - state: AtomicUsize::new(0), - }; - /// Waits until a value is written into the slot. fn wait_write(&self) { let backoff = Backoff::new(); @@ -62,12 +58,27 @@ struct Block { } impl Block { - /// Creates an empty block that starts at `start_index`. - fn new() -> Block { - Self { - next: AtomicPtr::new(ptr::null_mut()), - slots: [Slot::UNINIT; BLOCK_CAP], + /// Creates an empty block. + fn new() -> Box { + let layout = Layout::new::(); + assert!( + layout.size() != 0, + "Block should never be zero-sized, as it has an AtomicPtr field" + ); + // SAFETY: layout is not zero-sized + let ptr = unsafe { alloc_zeroed(layout) }; + // Handle allocation failure + if ptr.is_null() { + handle_alloc_error(layout) } + // SAFETY: This is safe because: + // [1] `Block::next` (AtomicPtr) may be safely zero initialized. + // [2] `Block::slots` (Array) may be safely zero initialized because of [3, 4]. + // [3] `Slot::value` (UnsafeCell) may be safely zero initialized because it + // holds a MaybeUninit. + // [4] `Slot::state` (AtomicUsize) may be safely zero initialized. + // TODO: unsafe { Box::new_zeroed().assume_init() } + unsafe { Box::from_raw(ptr.cast()) } } /// Waits until the next pointer is set. @@ -209,12 +220,12 @@ impl SegQueue { // If we're going to have to install the next block, allocate it in advance in order to // make the wait for other threads as short as possible. if offset + 1 == BLOCK_CAP && next_block.is_none() { - next_block = Some(Box::new(Block::::new())); + next_block = Some(Block::::new()); } // If this is the first push operation, we need to allocate the first block. if block.is_null() { - let new = Box::into_raw(Box::new(Block::::new())); + let new = Box::into_raw(Block::::new()); if self .tail diff --git a/crossbeam-queue/tests/seg_queue.rs b/crossbeam-queue/tests/seg_queue.rs index bf5fb998a..d2ad1e472 100644 --- a/crossbeam-queue/tests/seg_queue.rs +++ b/crossbeam-queue/tests/seg_queue.rs @@ -193,3 +193,18 @@ fn into_iter_drop() { assert_eq!(i, j); } } + +// If `Block` is created on the stack, the array of slots will multiply this `BigStruct` and +// probably overflow the thread stack. It's now directly created on the heap to avoid this. +#[test] +fn stack_overflow() { + const N: usize = 32_768; + struct BigStruct { + _data: [u8; N], + } + + let q = SegQueue::new(); + q.push(BigStruct { _data: [0u8; N] }); + + for _data in q.into_iter() {} +} From c711a116bb6d0defd70f2c97126e178a0843c7f0 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 15 Dec 2024 23:27:35 +0900 Subject: [PATCH 17/18] Calculate layout in const context --- crossbeam-channel/src/flavors/list.rs | 12 ++++++++---- crossbeam-deque/src/deque.rs | 12 ++++++++---- crossbeam-queue/src/seg_queue.rs | 12 ++++++++---- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/crossbeam-channel/src/flavors/list.rs b/crossbeam-channel/src/flavors/list.rs index 9f2e55258..6c15991f9 100644 --- a/crossbeam-channel/src/flavors/list.rs +++ b/crossbeam-channel/src/flavors/list.rs @@ -72,18 +72,22 @@ struct Block { } impl Block { - /// Creates an empty block. - fn new() -> Box { + const LAYOUT: Layout = { let layout = Layout::new::(); assert!( layout.size() != 0, "Block should never be zero-sized, as it has an AtomicPtr field" ); + layout + }; + + /// Creates an empty block. + fn new() -> Box { // SAFETY: layout is not zero-sized - let ptr = unsafe { alloc_zeroed(layout) }; + let ptr = unsafe { alloc_zeroed(Self::LAYOUT) }; // Handle allocation failure if ptr.is_null() { - handle_alloc_error(layout) + handle_alloc_error(Self::LAYOUT) } // SAFETY: This is safe because: // [1] `Block::next` (AtomicPtr) may be safely zero initialized. diff --git a/crossbeam-deque/src/deque.rs b/crossbeam-deque/src/deque.rs index 27354e09e..84ba0b8c0 100644 --- a/crossbeam-deque/src/deque.rs +++ b/crossbeam-deque/src/deque.rs @@ -1225,18 +1225,22 @@ struct Block { } impl Block { - /// Creates an empty block. - fn new() -> Box { + const LAYOUT: Layout = { let layout = Layout::new::(); assert!( layout.size() != 0, "Block should never be zero-sized, as it has an AtomicPtr field" ); + layout + }; + + /// Creates an empty block. + fn new() -> Box { // SAFETY: layout is not zero-sized - let ptr = unsafe { alloc_zeroed(layout) }; + let ptr = unsafe { alloc_zeroed(Self::LAYOUT) }; // Handle allocation failure if ptr.is_null() { - handle_alloc_error(layout) + handle_alloc_error(Self::LAYOUT) } // SAFETY: This is safe because: // [1] `Block::next` (AtomicPtr) may be safely zero initialized. diff --git a/crossbeam-queue/src/seg_queue.rs b/crossbeam-queue/src/seg_queue.rs index 68197e896..ad8f9c1ab 100644 --- a/crossbeam-queue/src/seg_queue.rs +++ b/crossbeam-queue/src/seg_queue.rs @@ -58,18 +58,22 @@ struct Block { } impl Block { - /// Creates an empty block. - fn new() -> Box { + const LAYOUT: Layout = { let layout = Layout::new::(); assert!( layout.size() != 0, "Block should never be zero-sized, as it has an AtomicPtr field" ); + layout + }; + + /// Creates an empty block. + fn new() -> Box { // SAFETY: layout is not zero-sized - let ptr = unsafe { alloc_zeroed(layout) }; + let ptr = unsafe { alloc_zeroed(Self::LAYOUT) }; // Handle allocation failure if ptr.is_null() { - handle_alloc_error(layout) + handle_alloc_error(Self::LAYOUT) } // SAFETY: This is safe because: // [1] `Block::next` (AtomicPtr) may be safely zero initialized. From 05c6275c429b0fc4842fc2d13d7791a4d0dd725b Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 15 Dec 2024 23:53:53 +0900 Subject: [PATCH 18/18] Prepare for the next release - crossbeam-utils 0.8.20 -> 0.8.21 - crossbeam-channel 0.5.13 -> 0.5.14 - crossbeam-deque 0.8.5 -> 0.8.6 - crossbeam-queue 0.3.11 -> 0.3.12 --- crossbeam-channel/CHANGELOG.md | 7 +++++++ crossbeam-channel/Cargo.toml | 2 +- crossbeam-deque/CHANGELOG.md | 4 ++++ crossbeam-deque/Cargo.toml | 2 +- crossbeam-queue/CHANGELOG.md | 4 ++++ crossbeam-queue/Cargo.toml | 2 +- crossbeam-utils/CHANGELOG.md | 6 +++++- crossbeam-utils/Cargo.toml | 2 +- 8 files changed, 24 insertions(+), 5 deletions(-) diff --git a/crossbeam-channel/CHANGELOG.md b/crossbeam-channel/CHANGELOG.md index 4d7fa0f18..60ac0513f 100644 --- a/crossbeam-channel/CHANGELOG.md +++ b/crossbeam-channel/CHANGELOG.md @@ -1,3 +1,10 @@ +# Version 0.5.14 + +- Fix stack overflow when sending large value to unbounded channel. (#1146, #1147) +- Add `Select::new_biased` function. (#1150) +- Remove inefficient spinning. (#1154) +- Suppress buggy `clippy::zero_repeat_side_effects` lint in macro generated code. (#1123) + # Version 0.5.13 - Add `select_biased!` macro. (#1040) diff --git a/crossbeam-channel/Cargo.toml b/crossbeam-channel/Cargo.toml index 8d4adf8aa..2991e5736 100644 --- a/crossbeam-channel/Cargo.toml +++ b/crossbeam-channel/Cargo.toml @@ -4,7 +4,7 @@ name = "crossbeam-channel" # - Update CHANGELOG.md # - Update README.md (when increasing major or minor version) # - Run './tools/publish.sh crossbeam-channel ' -version = "0.5.13" +version = "0.5.14" edition = "2021" rust-version = "1.60" license = "MIT OR Apache-2.0" diff --git a/crossbeam-deque/CHANGELOG.md b/crossbeam-deque/CHANGELOG.md index 691d3526e..5bc6e9f47 100644 --- a/crossbeam-deque/CHANGELOG.md +++ b/crossbeam-deque/CHANGELOG.md @@ -1,3 +1,7 @@ +# Version 0.8.6 + +- Fix stack overflow when pushing large value to `Injector`. (#1146, #1147, #1159) + # Version 0.8.5 - Remove dependency on `cfg-if`. (#1072) diff --git a/crossbeam-deque/Cargo.toml b/crossbeam-deque/Cargo.toml index 37e5f784a..3c6d44639 100644 --- a/crossbeam-deque/Cargo.toml +++ b/crossbeam-deque/Cargo.toml @@ -4,7 +4,7 @@ name = "crossbeam-deque" # - Update CHANGELOG.md # - Update README.md (when increasing major or minor version) # - Run './tools/publish.sh crossbeam-deque ' -version = "0.8.5" +version = "0.8.6" edition = "2021" rust-version = "1.61" license = "MIT OR Apache-2.0" diff --git a/crossbeam-queue/CHANGELOG.md b/crossbeam-queue/CHANGELOG.md index 68c9bbd73..e19a63926 100644 --- a/crossbeam-queue/CHANGELOG.md +++ b/crossbeam-queue/CHANGELOG.md @@ -1,3 +1,7 @@ +# Version 0.3.12 + +- Fix stack overflow when pushing large value to `SegQueue`. (#1146, #1147, #1159) + # Version 0.3.11 - Remove dependency on `cfg-if`. (#1072) diff --git a/crossbeam-queue/Cargo.toml b/crossbeam-queue/Cargo.toml index 0cec3bdae..8a2d7cc90 100644 --- a/crossbeam-queue/Cargo.toml +++ b/crossbeam-queue/Cargo.toml @@ -4,7 +4,7 @@ name = "crossbeam-queue" # - Update CHANGELOG.md # - Update README.md (when increasing major or minor version) # - Run './tools/publish.sh crossbeam-queue ' -version = "0.3.11" +version = "0.3.12" edition = "2021" rust-version = "1.60" license = "MIT OR Apache-2.0" diff --git a/crossbeam-utils/CHANGELOG.md b/crossbeam-utils/CHANGELOG.md index 4da2deb0f..5aa1967e7 100644 --- a/crossbeam-utils/CHANGELOG.md +++ b/crossbeam-utils/CHANGELOG.md @@ -1,3 +1,7 @@ +# Version 0.8.21 + +- Improve implementation of `CachePadded`. (#1152) + # Version 0.8.20 - Implement `Display` for `CachePadded`. (#1097) @@ -29,7 +33,7 @@ - Add `#[clippy::has_significant_drop]` to `ShardedLock{Read,Write}Guard`. (#958) - Improve handling of very large timeout. (#953) -- Soft-deprecate `thread::scope()` in favor of the more efficient `std::thread::scope` that stabilized on Rust 1.63. (#954) +- Soft-deprecate `thread::scope()` in favor of the more efficient `std::thread::scope` that stabilized in Rust 1.63. (#954) # Version 0.8.14 diff --git a/crossbeam-utils/Cargo.toml b/crossbeam-utils/Cargo.toml index 955f8a87d..3a95baea2 100644 --- a/crossbeam-utils/Cargo.toml +++ b/crossbeam-utils/Cargo.toml @@ -4,7 +4,7 @@ name = "crossbeam-utils" # - Update CHANGELOG.md # - Update README.md (when increasing major or minor version) # - Run './tools/publish.sh crossbeam-utils ' -version = "0.8.20" +version = "0.8.21" edition = "2021" rust-version = "1.60" license = "MIT OR Apache-2.0"