Skip to content

Commit 9be9131

Browse files
committed
channel: Create list::Block directly on the heap
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: * rust-lang/rust#102246 * rust-lang/rust#132738
1 parent 10f0296 commit 9be9131

File tree

2 files changed

+28
-12
lines changed

2 files changed

+28
-12
lines changed

crossbeam-channel/src/flavors/list.rs

+13-12
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! Unbounded channel implemented as a linked list.
22
3+
use std::alloc::{alloc_zeroed, Layout};
34
use std::boxed::Box;
45
use std::cell::UnsafeCell;
56
use std::marker::PhantomData;
@@ -50,11 +51,6 @@ struct Slot<T> {
5051
}
5152

5253
impl<T> Slot<T> {
53-
const UNINIT: Self = Self {
54-
msg: UnsafeCell::new(MaybeUninit::uninit()),
55-
state: AtomicUsize::new(0),
56-
};
57-
5854
/// Waits until a message is written into the slot.
5955
fn wait_write(&self) {
6056
let backoff = Backoff::new();
@@ -77,11 +73,16 @@ struct Block<T> {
7773

7874
impl<T> Block<T> {
7975
/// Creates an empty block.
80-
fn new() -> Self {
81-
Self {
82-
next: AtomicPtr::new(ptr::null_mut()),
83-
slots: [Slot::UNINIT; BLOCK_CAP],
84-
}
76+
fn new() -> Box<Self> {
77+
// SAFETY: This is safe because:
78+
// [1] `Block::next` (AtomicPtr) may be safely zero initialized.
79+
// [2] `Block::slots` (Array) may be safely zero initialized because of [3, 4].
80+
// [3] `Slot::msg` (UnsafeCell) may be safely zero initialized because it
81+
// holds a MaybeUninit.
82+
// [4] `Slot::state` (AtomicUsize) may be safely zero initialized.
83+
// TODO: unsafe { Box::new_zeroed().assume_init() }
84+
let layout = Layout::new::<Self>();
85+
unsafe { Box::from_raw(alloc_zeroed(layout).cast()) }
8586
}
8687

8788
/// Waits until the next pointer is set.
@@ -223,13 +224,13 @@ impl<T> Channel<T> {
223224
// If we're going to have to install the next block, allocate it in advance in order to
224225
// make the wait for other threads as short as possible.
225226
if offset + 1 == BLOCK_CAP && next_block.is_none() {
226-
next_block = Some(Box::new(Block::<T>::new()));
227+
next_block = Some(Block::<T>::new());
227228
}
228229

229230
// If this is the first message to be sent into the channel, we need to allocate the
230231
// first block and install it.
231232
if block.is_null() {
232-
let new = Box::into_raw(Box::new(Block::<T>::new()));
233+
let new = Box::into_raw(Block::<T>::new());
233234

234235
if self
235236
.tail

crossbeam-channel/tests/list.rs

+15
Original file line numberDiff line numberDiff line change
@@ -580,3 +580,18 @@ fn channel_through_channel() {
580580
})
581581
.unwrap();
582582
}
583+
584+
// If `Block` is created on the stack, the array of slots will multiply this `BigStruct` and
585+
// probably overflow the thread stack. It's now directly created on the heap to avoid this.
586+
#[test]
587+
fn stack_overflow() {
588+
const N: usize = 32_768;
589+
struct BigStruct {
590+
_data: [u8; N],
591+
}
592+
593+
let (sender, receiver) = unbounded::<BigStruct>();
594+
sender.send(BigStruct { _data: [0u8; N] }).unwrap();
595+
596+
for _data in receiver.try_iter() {}
597+
}

0 commit comments

Comments
 (0)