Skip to content

Commit

Permalink
Use Box<[T]> instead of Vec<T> to initialize and drop ArrayQueue
Browse files Browse the repository at this point in the history
Previously, the queue buffer was initialized using Vec::with_capacity.
Note the with_capacity does not guarantee that precise capacity;
the actual layout of the allocated memory could have a greater size.

The drop code assumed the buffer has a size
equal to the queue's capacity. This is undefined
behavior when the buffer actually has a greater size.

Using Box<[T]> instead guarantees an exact capacity, resolving the UB.
  • Loading branch information
caelunshun committed Jun 26, 2020
1 parent 81ab18e commit 769c195
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 9 deletions.
12 changes: 8 additions & 4 deletions crossbeam-channel/src/flavors/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl<T> Channel<T> {
// Allocate a buffer of `cap` slots initialized
// with stamps.
let buffer = {
let mut v: Vec<Slot<T>> = (0..cap)
let mut boxed: Box<[Slot<T>]> = (0..cap)
.map(|i| {
// Set the stamp to `{ lap: 0, mark: 0, index: i }`.
Slot {
Expand All @@ -124,8 +124,8 @@ impl<T> Channel<T> {
}
})
.collect();
let ptr = v.as_mut_ptr();
mem::forget(v);
let ptr = boxed.as_mut_ptr();
mem::forget(boxed);
ptr
};

Expand Down Expand Up @@ -553,7 +553,11 @@ impl<T> Drop for Channel<T> {

// Finally, deallocate the buffer, but don't run any destructors.
unsafe {
Vec::from_raw_parts(self.buffer, 0, self.cap);
// Create a slice from the buffer to make
// a fat pointer. Then, use Box::from_raw
// to deallocate it.
let ptr = std::slice::from_raw_parts_mut(self.buffer, self.cap) as *mut [Slot<T>];
Box::from_raw(ptr);
}
}
}
Expand Down
14 changes: 9 additions & 5 deletions crossbeam-queue/src/array_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
//! - Simplified BSD License and Apache License, Version 2.0
//! - http://www.1024cores.net/home/code-license
use alloc::vec::Vec;
use alloc::boxed::Box;
use core::cell::UnsafeCell;
use core::fmt;
use core::marker::PhantomData;
Expand Down Expand Up @@ -110,7 +110,7 @@ impl<T> ArrayQueue<T> {
// Allocate a buffer of `cap` slots initialized
// with stamps.
let buffer = {
let mut v: Vec<Slot<T>> = (0..cap)
let mut boxed: Box<[Slot<T>]> = (0..cap)
.map(|i| {
// Set the stamp to `{ lap: 0, index: i }`.
Slot {
Expand All @@ -119,8 +119,8 @@ impl<T> ArrayQueue<T> {
}
})
.collect();
let ptr = v.as_mut_ptr();
mem::forget(v);
let ptr = boxed.as_mut_ptr();
mem::forget(boxed);
ptr
};

Expand Down Expand Up @@ -425,7 +425,11 @@ impl<T> Drop for ArrayQueue<T> {

// Finally, deallocate the buffer, but don't run any destructors.
unsafe {
Vec::from_raw_parts(self.buffer, 0, self.cap);
// Create a slice from the buffer to make
// a fat pointer. Then, use Box::from_raw
// to deallocate it.
let ptr = std::slice::from_raw_parts_mut(self.buffer, self.cap) as *mut [Slot<T>];
Box::from_raw(ptr);
}
}
}
Expand Down

0 comments on commit 769c195

Please sign in to comment.