Skip to content

Commit

Permalink
Explicitly zero consumer on take, reinitialise when both released
Browse files Browse the repository at this point in the history
  • Loading branch information
timvisee committed Nov 12, 2020
1 parent 4a37dd9 commit de43a30
Showing 1 changed file with 26 additions and 34 deletions.
60 changes: 26 additions & 34 deletions core/src/bbbuffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,6 @@ where
/// Attempt to take a `Producer` from the `BBBuffer` to gain access to the
/// buffer. If a producer has already been taken, an error will be returned.
///
/// NOTE: When splitting, the underlying buffer will be explicitly initialized
/// to zero. This may take a measurable amount of time, depending on the size
/// of the buffer. This is necessary to prevent undefined behavior. If the buffer
/// is placed at `static` scope within the `.bss` region, the explicit initialization
/// will be elided (as it is already performed as part of memory initialization)
///
/// NOTE: If the `thumbv6` feature is selected, this function takes a short critical section
/// while splitting.
pub fn try_take_producer(&'a self) -> Result<Producer<'a, N>> {
Expand All @@ -135,13 +129,6 @@ where
}

unsafe {
// TODO: do we need to zero buffer here, like try_split?
// // Explicitly zero the data to avoid undefined behavior.
// // This is required, because we hand out references to the buffers,
// // which mean that creating them as references is technically UB for now
// let mu_ptr = self.0.buf.get();
// (*mu_ptr).as_mut_ptr().write_bytes(0u8, 1);

let nn1 = NonNull::new_unchecked(self as *const _ as *mut _);

Ok(Producer {
Expand All @@ -154,7 +141,7 @@ where
/// Attempt to take a `Consumer` from the `BBBuffer` to gain access to the
/// buffer. If a consumer has already been taken, an error will be returned.
///
/// NOTE: When splitting, the underlying buffer will be explicitly initialized
/// NOTE: When taking the consumer, the underlying buffer will be explicitly initialized
/// to zero. This may take a measurable amount of time, depending on the size
/// of the buffer. This is necessary to prevent undefined behavior. If the buffer
/// is placed at `static` scope within the `.bss` region, the explicit initialization
Expand All @@ -169,12 +156,11 @@ where
}

unsafe {
// TODO: do we need to zero buffer here, like try_split?
// // Explicitly zero the data to avoid undefined behavior.
// // This is required, because we hand out references to the buffers,
// // which mean that creating them as references is technically UB for now
// let mu_ptr = self.0.buf.get();
// (*mu_ptr).as_mut_ptr().write_bytes(0u8, 1);
// Explicitly zero the data to avoid undefined behavior.
// This is required, because we hand out references to the buffers,
// which mean that creating them as references is technically UB for now
let mu_ptr = self.0.buf.get();
(*mu_ptr).as_mut_ptr().write_bytes(0u8, 1);

let nn1 = NonNull::new_unchecked(self as *const _ as *mut _);

Expand Down Expand Up @@ -281,8 +267,9 @@ where

/// Attempt to release the `Producer`.
///
/// This re-initializes the buffer so it may be split in a different mode at a later
/// time. There must be no read or write grants active, or an error will be returned.
/// This re-initializes the buffer if the consumer was already released so it may be
/// split in a different mode at a later time. There must be no read or write grants
/// active, or an error will be returned.
///
/// The `Producer` ust be from THIS `BBBuffer`, or an error will be returned.
///
Expand Down Expand Up @@ -338,11 +325,13 @@ where
// Drop the producer
drop(prod);

// Re-initialize the buffer (not totally needed, but nice to do)
self.0.write.store(0, Release);
self.0.read.store(0, Release);
self.0.reserve.store(0, Release);
self.0.last.store(0, Release);
// Re-initialize the buffer if consumer is already released (not totally needed, but nice to do)
if self.0.split_prod_cons.load(Release) & BIT_CONSUMER == 0 {
self.0.write.store(0, Release);
self.0.read.store(0, Release);
self.0.reserve.store(0, Release);
self.0.last.store(0, Release);
}

// Mark the buffer as ready to retake producer
atomic::fetch_and(&self.0.split_prod_cons, !BIT_PRODUCER, Release);
Expand All @@ -352,8 +341,9 @@ where

/// Attempt to release the `Consumer`.
///
/// This re-initializes the buffer so it may be split in a different mode at a later
/// time. There must be no read or write grants active, or an error will be returned.
/// This re-initializes the buffer if the producer was already released so it may be
/// split in a different mode at a later time. There must be no read or write grants
/// active, or an error will be returned.
///
/// The `Consumer` must be from THIS `BBBuffer`, or an error will be returned.
///
Expand Down Expand Up @@ -409,11 +399,13 @@ where
// Drop the consumer
drop(cons);

// Re-initialize the buffer (not totally needed, but nice to do)
self.0.write.store(0, Release);
self.0.read.store(0, Release);
self.0.reserve.store(0, Release);
self.0.last.store(0, Release);
// Re-initialize the buffer if producer is already released (not totally needed, but nice to do)
if self.0.split_prod_cons.load(Release) & BIT_PRODUCER == 0 {
self.0.write.store(0, Release);
self.0.read.store(0, Release);
self.0.reserve.store(0, Release);
self.0.last.store(0, Release);
}

// Mark the buffer as ready to retake consumer
atomic::fetch_and(&self.0.split_prod_cons, !BIT_CONSUMER, Release);
Expand Down

0 comments on commit de43a30

Please sign in to comment.