Mutex.borrow_mut()

[surma]

Is there a reason Mutex does not have a borrow_mut()?

Happy to open a PR if desired :)

[hannobraun]

I think the reason is that you couldn't make that safe.

Either borrow_mut takes &self, which means you cam use it to produce multiple &mut references to the same thing, which is undefined behavior (more details). Or borrow_mut takes &mut self, but then you need a mutable static to store the Mutex, which can only be accessed using unsafe.

If you have a use case that requires using a Mutex to protect something mutable, you can do this by combining Mutex with core::RefCell.

[surma]

I think you are right. I got short-sighted here. Apologies :)

[hannobraun]

No need to apologize!

[surma]

Okay, after thinking about this for a while, I actually think Mutex should provide a borrow_mut().

Looking at the stdlib’s Mutex, you get to own a MutexGuard after getting a lock on the mutex — the equivalent of starting a critical section. Through the MutexGuard you can get a mutable reference to the mutex’d value via deref_mut().

Considering that, I think it makes sense to allow a mutable borrow even if you only have &self, as the usage of Mutex already guarantees that only one code path can have access to the mutex’d value (for the duration of the critical section).

What do you think?

[japaric]

@surma

bare_metal::Mutex doesn't have the same semantics that std::sync::Mutex has; in particular, the former never deadlocks.

Here's a std::sync::Mutex example that would break Rust aliasing rules if it didn't deadlock:

use std::sync::Mutex;

fn main() {
    let x = Mutex::new(0);

    let mut y = x.lock().unwrap();
    let refmut: &mut i32 = &mut *y;

    let mut z = x.lock().unwrap(); // this will deadlock
    let alias: &mut i32 = &mut *z; // oh no! another mutable reference
}

If we add a borrow_mut method to bare_metal::Mutex and change nothing else the following code would compile and result in undefined behavior:

static X: Mutex<i32> = Mutex::new(0);

fn ub() {
    interrupt::free(|cs: &CriticalSection| {
        let y: &mut i32 = X.borrow_mut(cs);

        let z: &mut i32 = X.borrow_mut(cs); // another mutable reference == UB
    });
}

It's not possible to have a safe borrow_mut method without adding some runtime check. If you add a runtime check you basically have the equivalent to today's Mutex<RefCell<T>>.

[surma]

Lol, borrowing twice in the same CS. You are absolutely right, of course. Thank you for explaining.

[cr1901]

@hannobraun core::RefCell unfortunately has unacceptable size overhead for me; it doubles the size of sample msp430 binaries I've written with interrupt handlers and rustc fails to optimize out dead code and strings when I use it.

Pending better optimization, would a mini RefCell without the formatting overhead be considered for bare metal (i.e. CriticalCell).

[devcexx]

Would making the free function provide a &mut CriticalSection instead of a &CriticalSection result into some unsafe behaviors? I was thinking that that mut pointer to the Critical Section might be used as a proof for preventing multiple borrows, something like:

pub fn borrow_mut<'cs>(&'cs self, _cs: &mut CriticalSection<'cs>) -> &'cs mut T {
    // ....
}

Of course, you will not be able to things like using that CriticalSection more than once at a time, either from a different mutex, or the same one, but at least you may have an escape hatch for the simpler situations that might prevent you for using RefCell or unsafe code.