Option holding Mutex containing RefCell... or Mutex holding Option... or something else?

Coming from C++, I am new to rust, and struggle with finding solutions for very basic things.
In an embedded project using embassy, I would like to store the Peripherals struct in a static variable that can be used for initialization of the corresponding periphery within several async tasks.

What I am trying so far is:

pub static mut PERIPHERALS: Option<Mutex<ThreadModeRawMutex, RefCell<embassy_stm32::Peripherals>>> = None;

The idea is that I need to protect access to the struct with a Mutex, and since the variable is only initialized later in main I think I need to wrap it in an Option. Or would it be better to have a Mutex wrapping an Option? (I initially tried this but ended up in [for me] syntax hell) I am asking this because later I get

    // ... now within 'main' ...
    let mut config = Config::default();
    // ... some clock settings within config done here...
    PERIPHERALS = Some(Mutex::new(RefCell::new(embassy_stm32::init(config))));

Here the compiler is unhappy because this is unsafe. I guess the reason is that I place a new Mutex there while there might already be one there (?).

Later in my async tasks I have stuff like this:

pub async fn some_task() {

    let &mut p = PERIPHERALS.unwrap().lock().await;
    // do something with p here...

And here again I get a message that this is unsafe...

I now wonder whether I actually am on a totally wrong track and what the best way is to achieve this.

Mutable static variables are inherently unsafe, because there is no protection against multiple threads accessing the variable simultaneously.

You probably want something like:

pub static PERIPHERALS: Mutex<Option<etc etc>> = Mutex::new(None);

A Mutex provides safe mutability via a shared reference, so the static itself doesn't need to be mut. Also, you want the Option inside the Mutex so that the mutex protects it. If you did use a static mut and Option<Mutex<etc>>, you put yourself in a situation where there's no way to safely set the static in the first place (since the Option has no synchronisation).

No, Rust will not insert magic mutexes anywhere, ever, for any reason. It won't even secretly allocate behind your back. It's unsafe because static mut is always unsafe.

2 Likes

Ok, I understand (I think):

pub static PERIPHERALS: Mutex<ThreadModeRawMutex, Option<embassy_stm32::Peripherals>> = Mutex::new(None);

And to initialize the peripherals, I would use:

    // in main:
    let mut p = PERIPHERALS.lock().await;
    *p =  Some(embassy_stm32::init(config));

and in the task, for example

        let p = PERIPHERALS.lock().await;
        let _dp = Output::new(p.unwrap().PA12, Level::Low, Speed::Low);

But here I then get the next problem, namely

error[E0507]: cannot move out of dereference of `MutexGuard<'_, ThreadModeRawMutex, Option<Peripherals>>`
   --> src/main.rs:59:31
    |
59  |         let _dp = Output::new(p.unwrap().PA12, Level::Low, Speed::Low);
    |                               ^ -------- value moved due to this method call
    |                               |
    |                               help: consider calling `.as_ref()` or `.as_mut()` to borrow the type's contents
    |                               move occurs because value has type `Option<Peripherals>`, which does not implement the `Copy` trait
    |

Somehow I don't understand how I can pass that part of the peripheral to the function without causing that problem...

unwrap moves the value out of the Option (remember, Rust doesn't have the overloads for different situations you can find in C++) so what it's suggesting is to take a reference to the content with either as_ref or as_mut, depending on what you need. That will give you an Option with a reference that you can unwrap if you want to assume that it's always set to something.

Edit: if you want to move the value out of the mutex, you can use take. It's closer to move semantics in C++ and will leave None behind.

Generally in embedded rust the idea is to have one task or thread working on each peripheral (splitting up peripherals in the main function). Trying to put them all in one shared struct like you seem to be doing is just asking for unnecessary pain and suffering. (Interrupt handlers are a case where you might not be able to avoid something like this though.)

It is hard to give more specific recommendations than that without knowing more about the specific problem you are trying to solve.

And while I have done embedded things, I have yet to work with embassy, so there might be some subtleties there that I'm not aware of.

Thanks for the suggestions so far.

What I am trying to achieve is "translate" an existing firmware I have in C to Rust.

(this does not matter here, but basically now there are lots of state machines called from a superloop, which really calls for some concurrency. I played with Miro Samek's active object framework, and while I like the idea, to me this is a macro-heavy hack around C code that I did not like much. Rust and embassy's async tasks looked ideal for that, and I want to learn Rust anyway) .

As a starting point, I used the example here (actually more the usb example, but this one is simpler and the problem is the same):

Access to the peripherals is provided in the Peripherals struct that is returned in p (1st line in main in the example). Since my application has a number of async tasks that each handle a separate peripheral, but they all need the p struct for initialization, I wanted to place p behind a mutex, allowing the async tasks to initialize their specific hardware in sequence once they get a lock on the mutex.

The docs say that Peripherals contains the "hardware singletons" and provide access to e.g. GPIO pins.

Back to the earlier code: I am now able to get a ref to the Option's content using as_ref`, but fail with the next call because this seems to move the data out of the struct.

What I have now is this:

pub static PERIPHERALS: Mutex<ThreadModeRawMutex,
                              Option<embassy_stm32::Peripherals>> =
    Mutex::new(None);

#[embassy_executor::main]
async fn main(spawner: Spawner) {
    let mut config = Config::default();
  
    {
        let mut p = PERIPHERALS.lock().await;
        *p =  Some(embassy_stm32::init(config));
    }
   // ....
}

pub async fn usb_task() {

    {
        let mut p = PERIPHERALS.lock().await;
        let p2 = p.as_mut().unwrap();
        let dp_ = Output::new(p2.PA12, Level::Low, Speed::Low);
        // ....
    }
    // ...
}

The error I now get is:

error[E0507]: cannot move out of `p2.PA12` which is behind a mutable reference
  --> src/main.rs:66:31
   |
66 |         let _dp = Output::new(p2.PA12, Level::Low, Speed::Low);
   |                               ^^^^^^^ move occurs because `p2.PA12` has type `PA12`, which does not implement the `Copy` trait

I understand the rationale of "consuming" the GPIO pin when an Output is defined, but this seems not to work with my idea of providing the peripherals struct behind the Mutex (or I don't get how).

(Plus, what if I later would want to reconfigure the peripherals? This way it seem not possible to use the particular GPIO pin again. But I could live with that limitation for the moment.)

Don't do that. Such "translation" never ends well. Ask yourself what the firmware needs to do, and implement it in idiomatic Rust. Don't think about the "how" in C.

And if you need to initialize something lazily, use once_cell::sync::Lazy.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.