bare-metal: Mutex is not safe on multi-core systems

On a multi-core system, disabling interrupts does not prevent the other core from operating, and so values protected by a bare_metal::Mutex will be incorrectly marked Sync.

Since the overwhelming majority of embedded use cases are single-core, I propose putting a prominent warning in the Mutex docstring for now, and working to develop a safe multi-core extension to the Mutex which can be enabled with a feature gate. Probably something using an atomic to implement a spinlock on top of requiring a CriticalSection.

About this issue

Original URL
State: closed
Created 6 years ago
Comments: 24 (22 by maintainers)

Commits related to this issue

Merge #13 13: Add note about mutex unsafety on multi-core systems r=japaric a=adamgreig See #12. Co-authored-by: Adam Greig <adam@adamgreig.com> — committed to rust-embedded/bare-metal by bors[bot] 6 years ago

Most upvoted comments

This could still be easily handled by creating a new crate with the critical section mutex that, with the exception of you @therealprof, people don’t seem to believe belongs in this crate.

Only a small fraction of people actually chimed in here, so it’s a bit early to make such statements.

Indeed I don’t have any issues with the Mutex being here but I don’t have any problems with moving it either. I’m just pointing out the obvious that any change to a foundational crate like this needs to be planned and executed with extreme care.

We still don’t have the ability to do something like a crater run to ensure that we’re not accidentally causing major damage to the ecosystem, so I’d rather we treat with extreme caution.

Of course, none of this solves the fact that this Mutex is not safe on multi-core systems, but there also seems to be some consensus that it’s dubious at best to think that a reasonable multi-core safe Mutex can be implemented without hardware support.

Indeed.

therealprof on Jan 31, 2020

This issue was effectively closed by https://github.com/rust-embedded/wg/pull/419; the Mutex in bare-metal is only considered sound on single-core systems and some other abstraction will be required for multi-core systems.

adamgreig on Jun 9, 2020

There seems to be some confusion about what CriticalSection and Mutex provide here.

CriticalSection is just a token that guarantees, for the duration of its existence, that the current core is in a critical section (ie. has any interrupts that could preempt execution disabled). The contract of this type means that any safe code that constructs a CriticalSection without disabling interrupts is unsound.
Mutex then takes this no-interrupts guarantee to provide mutual exclusion. This is sound if we adopt https://github.com/rust-embedded/wg/pull/419, but not at the current state. It’s sound in any case if the data is only accessible from a single core.

I’ll improve the docs of CriticalSection to make its contract clearer.

jonas-schievink on Feb 9, 2020

@therealprof I was looking into this again this morning. It is not Mutex that is unsound for multiple cores. It’s using cortex_m::interrupt::free to provide the CriticalSection that is unsound on multi-core.

It is possible to implement a different mechanism to provide the CritcialSection that would be sound.

https://gist.github.com/rubberduck203/20415cb0bdc0726b2ebf0903e7193665

rubberduck203 on Feb 8, 2020