search

Amanieu / parking_lot

Compact and efficient synchronization primitives for Rust. Also provides an API for creating custom synchronization primitives.
Apache License 2.0
2.76k stars 217 forks source link

Interest in a `critical-section` impl of `lock-api::RawMutex`? #447

Closed jamesmunns closed 4 months ago

jamesmunns commented 4 months ago

Hi there,

In the embedded ecosystem, we use the critical-section crate from the embedded wg for portable implementation of CS based mutexes, typically when sharing data with interrupts or multiple thread/priority levels.

I'm looking into using lock-api as an alternative to some existing home-rolled traits with a similar goal, and was wondering if you'd be interested in adding an impl of the Mutex trait via the acquire and release methods provided from the c-s crate, behind a feature flag.

Having this in the lock-api crate would make it easier for this to be consumed downstream, and wouldn't require an extra wrapper type. I wanted to reach out and ask before opening a PR!

Amanieu commented 4 months ago

There is a significant difference between critical sections (as defined by the critical-section crate) and mutexes: in the case of a mutex, if the lock is already taken then you can block and wait for the owning thread to release the lock. This doesn't work for critical sections: either you get the lock immediately, or you're in a deadlock situation.

This leads to different, incompatible APIs. For example, critical_section::acquire can never fail and will always succeed in acquiring a critical section. This means that Mutex::lock doesn't actually protect the inner data from having multiple mutable references to it.

jamesmunns commented 4 months ago

Yep, this difference started to be more clear to me when I started to implement this, particularly that critical-section promises a re-entrant lock, while RawMutex promises an exclusive lock.

The intended use case for this is to provide an encapsulated blocking mutex that can be used within a non-blocking/async mutex, however I've realized in a chat discussion that the current embassy-sync::RawMutex gets around these issues by using a closure to enforce LIFO lock ordering, in a way that is acceptable for re-entrant locking.

So: yeah, unfortunately I agree now that there might be an impedence mismatch here. I'll look at this some more over the weekend, and close this if I'm not able to think of any way around this.

jamesmunns commented 4 months ago

Ended up making separate traits that matched what embassy-sync and maitake-sync: https://github.com/tosc-rs/scoped-mutex/

Thanks for the feedback!