remove mut on split_ref function

[robamu]

I don't know if this can be problematic I actually received this warning in clippy on the most recent stable compiler in my embedded project:

warning: creating a mutable reference to mutable static is discouraged
   --> flashloader/src/main.rs:155:51
    |
155 |         let (buf_prod_tc, buf_cons_tc) = unsafe { BUF_RB_TC.split_ref() };
    |                                                   ^^^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives

What I am doing is basically defining a StaticRb like this in my RTIC based application:

// Ring buffers to handling variable sized telemetry
static mut RINGBUF: Lazy<StaticRb<u8, RX_RING_BUF_SIZE>> =
    Lazy::new(StaticRb::<u8, RX_RING_BUF_SIZE>::default);

and then splitting it like this

            let (data_producer, data_consumer) = unsafe { RINGBUF.split_ref() };

to pass the handles to different RTIC tasks. I need the unsafe because the RINGBUF has to be defined as a static mut.
When I received the mut bound from the split_ref function, I can use a regular static and I can still split the static structure.

[robamu]

I'll try to fix the CI

[agerasev]

Thank you for your PR! But split_ref requires mutable reference intentionally.

It's unsafe to implement split_ref for immutable reference because then it can be called multiple times on the same ring buffer and therefore we get multiple producers and consumers pointing to the same ring buffer that is UB. It is safe to have only single pair of producer and consumer pointing to the same ring buffer at the same time. By using mutable reference in split_ref such guarantee automatically provided by Rust's borrow checker.

But the problem is to get mutable reference to static variable. Rust cannot ensure that you does this only once, so it's considered unsafe. So you need to either unsafely get mutable reference to static mut as you already did and simply silence Clippy warning, or use some runtime check for mutably borrowing static variables (like OnceMut used in this example).

[agerasev]

Also I see that you are using Lazy which internally uses spinlocks AFAIK. Please note that using spinlocks is not a good practice, especially in RTOS, as it can lead to deadlock (see https://matklad.github.io/2020/01/02/spinlocks-considered-harmful.html).

[robamu]

Thanks for the excellent explanation, that definitely makes sense. Concerning Lazy : I only call the split_mut function once at program initialization. So I think this should not be a problem anymore? According to matklad/once_cell#195 , critical-section is used. I don't know if critical-section spins, probably depends on the implementation..

I now found a cleaner solution for RTIC specifically: Instead of splitting the StaticRb, I simply keep the StaticRb in the Shared structure of RTIC ( see here: https://github.com/us-irs/va108xx-rs/blob/main/examples/rtic/src/bin/uart-echo-rtic.rs) . I don´t know if this is somehow less performant because I am locking a structure which is already designed for being shared among threads, but my flashloader app still seems to work without any issues and I am not using the split API anymore. With that solution, I don´t need static mut or any variables in global scope anymore.

I am wondering how StaticRb in an embassy app would look like when sharing between embassy tasks.. In embassy, there is no special support for concurrent mutable access, so I would guess something like a Mutex<RefCell<StaticRb<...>>> is required..

In any case, my issue is solved and this can be closed :)