Skip to content

Commit

Permalink
enforced owner to be Send + 'static
Browse files Browse the repository at this point in the history
  • Loading branch information
amunra committed Oct 23, 2024
1 parent 1d7ef85 commit 6271eff
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 21 deletions.
5 changes: 4 additions & 1 deletion src/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,10 @@ impl Bytes {
///
/// Note that converting [Bytes] constructed from an owner into a [BytesMut]
/// will always create a deep copy of the buffer into newly allocated memory.
pub fn from_owner<T: AsRef<[u8]>>(owner: T) -> Self {
pub fn from_owner<T>(owner: T) -> Self
where
T: AsRef<[u8]> + Send + 'static,
{
let owned = Box::new(Owned {
lifetime: OwnedLifetime {
ref_cnt: AtomicUsize::new(1),
Expand Down
36 changes: 16 additions & 20 deletions tests/test_bytes.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#![warn(rust_2018_idioms)]

use bytes::{Buf, BufMut, Bytes, BytesMut};
use std::cell::Cell;
use std::rc::Rc;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;

use std::usize;

Expand Down Expand Up @@ -1482,17 +1482,14 @@ fn split_to_empty_addr_mut() {
let _ = &buf[..];
}

// N.B.: Using `Rc` and `Cell` rather than `Arc` and `usize`
// since this forces the type to be !Send + !Sync, ensuring
// in this test that the owner remains a generic T.
#[derive(Clone)]
struct OwnedTester<const L: usize> {
buf: [u8; L],
drop_count: Rc<Cell<usize>>,
drop_count: Arc<AtomicUsize>,
}

impl<const L: usize> OwnedTester<L> {
fn new(buf: [u8; L], drop_count: Rc<Cell<usize>>) -> Self {
fn new(buf: [u8; L], drop_count: Arc<AtomicUsize>) -> Self {
Self { buf, drop_count }
}
}
Expand All @@ -1505,63 +1502,62 @@ impl<const L: usize> AsRef<[u8]> for OwnedTester<L> {

impl<const L: usize> Drop for OwnedTester<L> {
fn drop(&mut self) {
let current = self.drop_count.get();
self.drop_count.set(current + 1)
self.drop_count.fetch_add(1, Ordering::AcqRel);
}
}

#[test]
fn owned_basic() {
let buf: [u8; 5] = [1, 2, 3, 4, 5];
let drop_counter = Rc::new(Cell::new(0));
let drop_counter = Arc::new(AtomicUsize::new(0));
let owner = OwnedTester::new(buf, drop_counter.clone());
let b1 = Bytes::from_owner(owner);
assert_eq!(&buf[..], &b1[..]);
assert!(b1.is_unique());
let b2 = b1.clone();
assert!(!b1.is_unique());
assert!(!b2.is_unique());
assert_eq!(drop_counter.get(), 0);
assert_eq!(drop_counter.load(Ordering::Acquire), 0);
drop(b1);
assert_eq!(drop_counter.get(), 0);
assert_eq!(drop_counter.load(Ordering::Acquire), 0);
assert!(b2.is_unique());
let b3 = b2.slice(1..b2.len() - 1);
assert!(!b2.is_unique());
assert!(!b3.is_unique());
drop(b2);
assert_eq!(drop_counter.get(), 0);
assert_eq!(drop_counter.load(Ordering::Acquire), 0);
assert!(b3.is_unique());
drop(b3);
assert_eq!(drop_counter.get(), 1);
assert_eq!(drop_counter.load(Ordering::Acquire), 1);
}

#[test]
fn owned_to_mut() {
let buf: [u8; 10] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
let drop_counter = Rc::new(Cell::new(0));
let drop_counter = Arc::new(AtomicUsize::new(0));
let owner = OwnedTester::new(buf, drop_counter.clone());
let b1 = Bytes::from_owner(owner);

// Holding an owner will fail converting to a BytesMut,
// even when the bytes instance is unique.
assert!(b1.is_unique());
let b1 = b1.try_into_mut().unwrap_err();
assert_eq!(drop_counter.get(), 0);
assert_eq!(drop_counter.load(Ordering::Acquire), 0);

// That said, it's still possible, just not cheap.
let bm1: BytesMut = b1.into();
let new_buf = &bm1[..];
assert_eq!(new_buf, &buf[..]);

assert_eq!(drop_counter.get(), 1);
assert_eq!(drop_counter.load(Ordering::Acquire), 1);
drop(bm1);
assert_eq!(drop_counter.get(), 1);
assert_eq!(drop_counter.load(Ordering::Acquire), 1);
}

#[test]
fn owned_to_vec() {
let buf: [u8; 10] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9];
let drop_counter = Rc::new(Cell::new(0));
let drop_counter = Arc::new(AtomicUsize::new(0));
let owner = OwnedTester::new(buf, drop_counter.clone());
let b1 = Bytes::from_owner(owner);
assert!(b1.is_unique());
Expand All @@ -1572,5 +1568,5 @@ fn owned_to_vec() {
assert_eq!(&v1[..], &b1[..]);

drop(b1);
assert_eq!(drop_counter.get(), 1);
assert_eq!(drop_counter.load(Ordering::Acquire), 1);
}

0 comments on commit 6271eff

Please sign in to comment.