From a quick-ish glance at RingBuffer, it looks like head and tail could possibly be falsely shared.

However, I tried to improve it by padding the atomics so they'd be on different cache lines, wrote a threaded benchmark, and failed to produce something faster than the current implementation (in fact, my version was usually slightly slower in benchmarks).

So, I guess this issue is either:

• How is the current implementation getting around false sharing?
• Or, if you haven't explicitly thought about false sharing before, perhaps look into it and see if you can make ringbuf even faster :)

Hi. Nice library, thanks for putting effort into making this.

I think I've discovered an issue allowing use of uninitialized data from safe code in the read_from function:

The issue is that a (safe) implementation of Read can read the uninitialized data in the buffer. This test:

#[test]
fn test_read_touching_buf() {

    struct Reader;

    impl Read for Reader {
        fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
            assert!(buf[0] == 0);
            Ok(0)
        }
    }

    let (mut producer, _consumer) = HeapRb::<u8>::new(10).split();
    producer.read_from(&mut Reader, None).unwrap();
}

Triggers this error when executing in Miri:

test foo ... error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
  --> src/main.rs:73:21
   |
73 |             assert!(buf[0] == 0);
   |                     ^^^^^^ using uninitialized data, but this operation requires initialized memory
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information

The behaviour of discard is currently too vague as documented, though I suspect this is a documentation issue.

It documentation currently offers no guarantees about how many, if any, elements will be discarded. In a situation in which I know for sure that there will be no elements added to the FIFO, to adhere to the spec I still need a loop that may or may not run forever.

In my use case, in which the producer is not producing and I want to empty the fifo, it's clear from the implementation that I don't need a loop at all, and just a calling discard() with Consumer::capacity() will reliably empty the fifo.

This is a situation where a doc-test would be extremely valuable, showing in which situation it may discard less than n (i.e. when n is larger than the current number of elements or in which there is a producer race). I'm happy to contribute this if it is the desired policy for discard.

A basic performance comparison would be great to compare against other rust ringbuffers, as well as crossbeam channel.

Hi,

I would like to overwrite when the buffer is full. Can that be done without using the consumer to remove values?

/Peter

Hello,

Would it be possible to introduce an overwriting mode where if the consumer gets too far behind it'll simply lose those unhandled values?

Thanks in advance

I have a use-case where I have a list of incoming events and want to retain the latest n items -- the producer pushes elements one by one, but the consumer will always iterate or fetch the full list without ever calling pop.

I'd like to know if there's a way for the producer to continue pushing items to the buffer, discarding old items if the buffer is full. Currently it seems the only way to do this is to have some coordination between the producer and consumer (check if is_full and then have the consumer pop, then the producer can push).

Not sure whether this fits with the design of this library, but is it feasible to have a version of push that overrides the oldest element if the buffer is full (or some other recommended approach)? On the consumer side, I'm not too concerned with receiving the items in the proper order, if that matters.

Pretty sure there's no reason not to, since they're in core: https://doc.rust-lang.org/core/fmt/index.html

I run Miri on your crate and it gave an UB on push_access function. I don't have a clear understanding on stacked borrows yet, so I don't know how it can be fixed.

This is the output:

% cargo +nightly miri test
   Compiling ringbuf v0.2.7 (/home/oblique/git/ringbuf)
    Finished test [unoptimized + debuginfo] target(s) in 0.09s
     Running unittests src/lib.rs (target/miri/x86_64-unknown-linux-gnu/debug/deps/ringbuf-e44ea49f80c6b586)

running 39 tests
test tests::access::discard ... error: Undefined Behavior: trying to reborrow <220885> for SharedReadWrite permission at alloc84260[0x0], but that tag does not exist in the borrow stack for this location
   --> src/producer.rs:99:19
    |
99  |         let n = f(slices.0, slices.1);
    |                   ^^^^^^^^
    |                   |
    |                   trying to reborrow <220885> for SharedReadWrite permission at alloc84260[0x0], but that tag does not exist in the borrow stack for this location
    |                   this error occurs as part of a reborrow at alloc84260[0x0..0xa]
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information

    = note: inside `producer::Producer::<i8>::push_access::<[closure@src/producer.rs:152:30: 159:14]>` at src/producer.rs:99:19
note: inside `producer::Producer::<i8>::push` at src/producer.rs:152:13
   --> src/producer.rs:152:13
    |
152 | /             self.push_access(|slice, _| {
153 | |                 if !slice.is_empty() {
154 | |                     mem::swap(slice.get_unchecked_mut(0), &mut elem_mu);
155 | |                     1
...   |
158 | |                 }
159 | |             })
    | |______________^
note: inside `tests::access::discard` at src/tests/access.rs:245:9
   --> src/tests/access.rs:245:9
    |
245 |         prod.push(i).unwrap();
    |         ^^^^^^^^^^^^
note: inside closure at src/tests/access.rs:237:1
   --> src/tests/access.rs:237:1
    |
236 |   #[test]
    |   ------- in this procedural macro expansion
237 | / fn discard() {
238 | |     // Initialize ringbuffer, prod and cons
239 | |     let rb = RingBuffer::<i8>::new(10);
240 | |     let (mut prod, mut cons) = rb.split();
...   |
279 | |     assert_eq!(cons.pop(), Some(0));
280 | | }
    | |_^
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error

error: test failed, to rerun pass '--lib'

Hi @agerasev ,

Question for you: is there a type alias to use when using a LocalRb for Producer?

HeapRb has HeapProducer but cannot figure out the correct type for a Producer of LocalRb

I want to use Mutex<LocalRb<T, [MaybeUninit<T>, N]>> with static variable but there is no initializer method for LocalRb<T, [MaybeUninit<T>, N]>. Could you provide something like LocalRb<T, [MaybeUninit<T>, N]>::new_const()?

PS: I found another crate that supports this

Is something like this safe?

static mut rb: StaticRb<u8, BUFFER_LEN> = StaticRb::<u8, BUFFER_LEN>::default();
let (mut prod, mut cons) = unsafe { rb.split_ref() };

My use-case is I want to have a ring buffer on a no std environment, and I would prefer not to use dynamic allocation if possible. The producer would be used by an interrupt handler, and the consumer used by the main task.

Currently HeapRb::new uses Vec::resize_with, which will panic if an allocation fails.

I propose adding a second function HeapRb::try_new which will return Result<Self, std::collections::TryReserveError> and uses the same logic as HeapRb::new but additionally calls Vec::try_reserve before the call to Vec::resize_with.

This would be helpful for embedded development where memory is limited and allocations may fail.

Seems like the master branch async ringbuf is not yet complete? There is an async index.rs file not yet included in lib.rs.

Currently, the close() of AsyncProd and AsyncCons does not call wake() which can cause some problem. I see that close() of AsyncIndex calls wake().

The v0.1.3 Async-Ringbuf also wakes the other on close().

When will this be implemented?

In the Write and Read implementations, calling Error::new() unnecessarily allocates memory (AFAIK):

To avoid this, std::io::ErrorKind::WouldBlock.into() can be used for error type conversion: https://doc.rust-lang.org/std/io/struct.Error.html#impl-From%3CErrorKind%3E

Intended for use for errors not exposed to the user, where allocating onto the heap (for normal construction via Error::new) is too costly.

This crate is non-blocking: when the buffer is full/empty, it returns Err(WouldBlock). I'd like to have an option with which the buffer blocks until more data/space is available. I can emulate this from the outside, but only with busy waiting, which I'd like to avoid.

When attempting to upgrade from version 0.3.3 to 0.4.0 I ran into this problem, where the recommended HeapRb<T> now requires T to be Sync. The following code generates 2 similar compiler errors:

fn is_send<T: Send>() {}
is_send::<HeapCons<Cell<i32>>>();
is_send::<HeapProd<Cell<i32>>>();

error[E0277]: `std::cell::Cell<i32>` cannot be shared between threads safely
  --> adae\src\engine\utils\ringbuffer.rs:13:15
   |
13 |     is_send::<HeapCons<Cell<i32>>>();
   |               ^^^^^^^^^^^^^^^^^^^ `std::cell::Cell<i32>` cannot be shared between threads safely
   |
   = help: the trait `std::marker::Sync` is not implemented for `std::cell::Cell<i32>`, which is required by `ringbuf::wrap::caching::Caching<std::sync::Arc<ringbuf::SharedRb<ringbuf::storage::Heap<std::cell::Cell<i32>>>>, false, true>: std::marker::Send`
   = note: if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead
   = note: required for `ringbuf::storage::Heap<std::cell::Cell<i32>>` to implement `std::marker::Sync`
note: required because it appears within the type `ringbuf::SharedRb<ringbuf::storage::Heap<std::cell::Cell<i32>>>`
  --> C:\Users\Holger\.cargo\registry\src\index.crates.io-6f17d22bba15001f\ringbuf-0.4.0\src\rb\shared.rs:47:12
   |
47 | pub struct SharedRb<S: Storage + ?Sized> {
   |            ^^^^^^^^
   = note: required for `std::sync::Arc<ringbuf::SharedRb<ringbuf::storage::Heap<std::cell::Cell<i32>>>>` to implement `std::marker::Send`
note: required because it appears within the type `ringbuf::wrap::frozen::Frozen<std::sync::Arc<ringbuf::SharedRb<ringbuf::storage::Heap<std::cell::Cell<i32>>>>, false, true>`
  --> C:\Users\Holger\.cargo\registry\src\index.crates.io-6f17d22bba15001f\ringbuf-0.4.0\src\wrap\frozen.rs:22:12
   |
22 | pub struct Frozen<R: RbRef, const P: bool, const C: bool> {
   |            ^^^^^^
note: required because it appears within the type `ringbuf::wrap::caching::Caching<std::sync::Arc<ringbuf::SharedRb<ringbuf::storage::Heap<std::cell::Cell<i32>>>>, false, true>`
  --> C:\Users\Holger\.cargo\registry\src\index.crates.io-6f17d22bba15001f\ringbuf-0.4.0\src\wrap\caching.rs:17:12
   |
17 | pub struct Caching<R: RbRef, const P: bool, const C: bool> {
   |            ^^^^^^^
note: required by a bound in `engine::utils::ringbuffer::test_is_sync::is_send`
  --> adae\src\engine\utils\ringbuffer.rs:12:19
   |
12 |     fn is_send<T: Send>() {}
   |                   ^^^^ required by this bound in `is_send`

This seems to originate from the use of Arc within HeapProd and HeapCons.

Is this expected behavior?