Skip to content

allow to construct BytesMut with custom alignment #601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions src/bytes_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,65 @@ impl BytesMut {
pub fn with_capacity(capacity: usize) -> BytesMut {
BytesMut::from_vec(Vec::with_capacity(capacity))
}

/// Creates a new `BytesMut` with the specified capacity and alignment.
///
/// The returned `BytesMut` will have alignment of `align` and be able to
/// hold at least `capacity` bytes without reallocating.
///
/// It is important to note that this function does not specify the length
/// of the returned `BytesMut`, but only the capacity.
///
/// # Examples
///
/// ```
/// use bytes::BytesMut;
///
/// let bytes = BytesMut::with_capacity_aligned(65000, 4096);
///
/// // `bytes` contains no data, even though there is capacity
/// assert_eq!(bytes.len(), 0);
/// // round up to match alignment
/// assert_eq!(bytes.capacity(), 65536);
/// ```
///
/// # Panics
/// + If `align` is zero or not power of two by
/// [Layout::from_size_align](std::alloc::Layout::from_size_align)
/// + If `capacity` round up to `align` is excess `isize::MAX` by
/// [Layout::from_size_align](std::alloc::Layout::from_size_align)
/// + If out of memory by [GlobalAlloc::alloc](std::alloc::GlobalAlloc::alloc)
#[inline]
pub fn with_capacity_aligned(capacity: usize, align: usize) -> BytesMut {
// if align isn't power of two it will fail in `std::alloc::Layout::from_size_align`
let lower = align - 1;
// round up to nearest number that aligned with `align`,
// if itself aligned this does nothing
let capacity = (capacity + lower) & !lower;
// should be
assert_eq!(capacity % align, 0);
let layout = std::alloc::Layout::from_size_align(capacity, align).unwrap();

let vec = unsafe {
// # Safety
// + capacity and alignment has been checked above
// + null ptr handled below
let ptr = std::alloc::alloc(layout);
if ptr.is_null() {
// handle null ptr here because it might cause a problem at `BytesMut::from_vec`
panic!("Not enough memory");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should call alloc::alloc::handle_alloc_error to handle OOM errors, since panicking may cause unexpected results in low-memory environments.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok thanks.

}
// # Safety
// + as long as `capacity % align == 0` it prevent `RawVec::drop` to cause `SIGSEGV`
// because `RawVec::drop` will create new Layout during free,
// `RawVec<u8>` will use `Layout::<u8>::array(capacity)`
// to free instead of our alignment
Vec::from_raw_parts(ptr, 0, capacity)
Comment on lines +193 to +198
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation for from_raw_parts says the following:

T needs to have the same alignment as what ptr was allocated with. (T having a less strict alignment is not sufficient, the alignment really needs to be equal to satisfy the dealloc requirement that memory must be allocated and deallocated with the same layout.)

Since the vector is using alignment 1, passing it a pointer of larger alignment is incorrect.

Copy link
Author

@Wireless4024 Wireless4024 Feb 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I see errors in miri.
and now I don't think it's possible without custom drop handling,
like making list of structs with alignment by list of power of two e.g. #[repr(align($align))] struct Align$align([0u8;$align]) and then pass custom struct on BytesMut::drop to make it aligned, and should I make it?
if you don't think please close my pr and my last question: can you make BytesMut::from_vec public to allow me to pass my illegal vec please?

};
// if `BytesMut::from_vec` is deprecated, please use ptr and capacity
// to construct `BytesMut` instead
BytesMut::from_vec(vec)
}

/// Creates a new `BytesMut` with default capacity.
///
Expand Down