Skip to content

Commit 9a0a2cf

Browse files
timviseeithinuel
authored andcommitted
Explicitly zero producer on take, reinitialise when both released
See: - #78 (comment) - #78 (comment)
1 parent 324806b commit 9a0a2cf

File tree

1 file changed

+26
-34
lines changed

1 file changed

+26
-34
lines changed

core/src/bbbuffer.rs

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ impl<'a, const N: usize> BBBuffer<N> {
146146
/// Attempt to take a `Producer` from the `BBBuffer` to gain access to the
147147
/// buffer. If a producer has already been taken, an error will be returned.
148148
///
149-
/// NOTE: When splitting, the underlying buffer will be explicitly initialized
149+
/// NOTE: When taking the producer, the underlying buffer will be explicitly initialized
150150
/// to zero. This may take a measurable amount of time, depending on the size
151151
/// of the buffer. This is necessary to prevent undefined behavior. If the buffer
152152
/// is placed at `static` scope within the `.bss` region, the explicit initialization
@@ -161,12 +161,11 @@ impl<'a, const N: usize> BBBuffer<N> {
161161
}
162162

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

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

@@ -180,12 +179,6 @@ impl<'a, const N: usize> BBBuffer<N> {
180179
/// Attempt to take a `Consumer` from the `BBBuffer` to gain access to the
181180
/// buffer. If a consumer has already been taken, an error will be returned.
182181
///
183-
/// NOTE: When splitting, the underlying buffer will be explicitly initialized
184-
/// to zero. This may take a measurable amount of time, depending on the size
185-
/// of the buffer. This is necessary to prevent undefined behavior. If the buffer
186-
/// is placed at `static` scope within the `.bss` region, the explicit initialization
187-
/// will be elided (as it is already performed as part of memory initialization)
188-
///
189182
/// NOTE: If the `thumbv6` feature is selected, this function takes a short critical section
190183
/// while splitting.
191184
pub fn try_take_consumer(&'a self) -> Result<Consumer<'a, N>> {
@@ -195,13 +188,6 @@ impl<'a, const N: usize> BBBuffer<N> {
195188
}
196189

197190
unsafe {
198-
// TODO: do we need to zero buffer here, like try_split?
199-
// // Explicitly zero the data to avoid undefined behavior.
200-
// // This is required, because we hand out references to the buffers,
201-
// // which mean that creating them as references is technically UB for now
202-
// let mu_ptr = self.buf.get();
203-
// (*mu_ptr).as_mut_ptr().write_bytes(0u8, 1);
204-
205191
let nn1 = NonNull::new_unchecked(self as *const _ as *mut _);
206192

207193
Ok(Consumer {
@@ -311,8 +297,9 @@ impl<'a, const N: usize> BBBuffer<N> {
311297

312298
/// Attempt to release the `Producer`.
313299
///
314-
/// This re-initializes the buffer so it may be split in a different mode at a later
315-
/// time. There must be no read or write grants active, or an error will be returned.
300+
/// This re-initializes the buffer if the consumer was already released so it may be
301+
/// split in a different mode at a later time. There must be no read or write grants
302+
/// active, or an error will be returned.
316303
///
317304
/// The `Producer` ust be from THIS `BBBuffer`, or an error will be returned.
318305
///
@@ -368,11 +355,13 @@ impl<'a, const N: usize> BBBuffer<N> {
368355
// Drop the producer
369356
drop(prod);
370357

371-
// Re-initialize the buffer (not totally needed, but nice to do)
372-
self.write.store(0, Release);
373-
self.read.store(0, Release);
374-
self.reserve.store(0, Release);
375-
self.last.store(0, Release);
358+
// Re-initialize the buffer if consumer is already released (not totally needed, but nice to do)
359+
if self.split_prod_cons.load(Acquire) & BIT_CONSUMER == 0 {
360+
self.write.store(0, Release);
361+
self.read.store(0, Release);
362+
self.reserve.store(0, Release);
363+
self.last.store(0, Release);
364+
}
376365

377366
// Mark the buffer as ready to retake producer
378367
atomic::fetch_and(&self.split_prod_cons, !BIT_PRODUCER, Release);
@@ -382,8 +371,9 @@ impl<'a, const N: usize> BBBuffer<N> {
382371

383372
/// Attempt to release the `Consumer`.
384373
///
385-
/// This re-initializes the buffer so it may be split in a different mode at a later
386-
/// time. There must be no read or write grants active, or an error will be returned.
374+
/// This re-initializes the buffer if the producer was already released so it may be
375+
/// split in a different mode at a later time. There must be no read or write grants
376+
/// active, or an error will be returned.
387377
///
388378
/// The `Consumer` must be from THIS `BBBuffer`, or an error will be returned.
389379
///
@@ -439,11 +429,13 @@ impl<'a, const N: usize> BBBuffer<N> {
439429
// Drop the consumer
440430
drop(cons);
441431

442-
// Re-initialize the buffer (not totally needed, but nice to do)
443-
self.write.store(0, Release);
444-
self.read.store(0, Release);
445-
self.reserve.store(0, Release);
446-
self.last.store(0, Release);
432+
// Re-initialize the buffer if producer is already released (not totally needed, but nice to do)
433+
if self.split_prod_cons.load(Acquire) & BIT_PRODUCER == 0 {
434+
self.write.store(0, Release);
435+
self.read.store(0, Release);
436+
self.reserve.store(0, Release);
437+
self.last.store(0, Release);
438+
}
447439

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

0 commit comments

Comments
 (0)