When using a Mutex<()> in low-level unsafe code, keeping around the guard in order to unlock can be tedious/suboptimal (and poisoning can be useless and thus suboptimal in panic-free code).
It's unfortunate because std already embeds all the code for a low-level mutex (sys::Mutex), which could just be exposed as is. That's why I would like to add a std::sync::lowlevel::Mutex with the following API:
pub struct Mutex {
inner: sys::Mutex
}
impl Mutex {
pub const fn new() -> Self {
Self { inner: sys::Mutex::new() }
}
pub fn try_lock(&self) -> bool {
self.inner.try_lock()
}
pub fn lock(&self) {
self.inner.lock();
}
/// # Safety
///
/// The mutex must be locked by the current thread.
pub unsafe fn unlock(&self) {
// SAFETY: same precondition
unsafe { self.inner.unlock(); }
}
}
The alternative would be to wait for std::sync::nonpoison::Mutex<()> and add an unsafe unlock to it, but I think a dedicated type better reflects the intent. And this lowlevel module could be extended with unsafe RwLock and CondVar.
Do you think it's worth an RFC?
EDIT: Another non-negligible advantage of exposing low-level methods is to make stdlib's mutex implementation compatible with lock_api ecosystem.
I don't think this API is a good idea. Forgetting to clean up resources, including unlocking mutexes, on some error paths is a very common error in C code. The guard pattern (also appearing as RAII in C++) solves this common footgun.
I can't think of any case where your suggestion improves the code quality and doesn't increase the risk of errors. When suggesting language or library features you really need to provide motivating use cases, which I think would be the next step if you feel this is worth persuing.
Yes, though you may want lock to be unsafe as well to prevent double locking. I am not sure if the implementation of the underlying mutex primitive on any target triple considers double-locking to be UB. It is UB for C++ and pthread at least.
OP is talking about low-level/unsafe applications, so I assume they are in a situation where they need to store the lock status out-of-band, without tying its lifetime to the mutex handle. There is no workaround here that I am aware of.
Raw pointers manipulation are also a very common error factor in C code, yet Rust expose them.
Because the main goal is code optimality, where useless assembly instructions are not wanted.
Let me gives you a concrete example: GitHub - wyfo/aiq: A concurrent intrusive queue for building async primitives · GitHub, this is a concurrent intrusive linked-list with lock-free insertion which uses a mutex for the removal part. The crate is #[no_std], so std::sync::Mutex is just one of the possible implementations for the mutex enabled with a cfg-flag, other ones like pthread-based are lower-level without RAII. The nodes data are wrapped in UnsafePinned, accessed through atomic pointers, and the mutex is only used for algorithmic guarantee.
To expose a safe API, I've a LockedList guard which unlocks the mutex on drop, so I'm using RAII (just a crafted one, but I've no choice because I supports mutexes without RAII). However, this guard embeds a reference to the list (storing the mutex in a field), and in the case of the std backend, it must also stores the mutex' guard, which also embeds a reference to the mutex. So my guard contains two redundant references instead of one (and I don't even talk about the poison guard which adds another word).
They may argue that this example is just two useless words in a struct, but it may have unwanted side effects, for example making the struct too big to fit in registers when passed to a cold function with other arguments.
Again, for the same reason Rust allows raw pointers manipulation, I don't see any reason the language should prevent any kind of low-level optimization when they are sound. And the code already exists in stdlib.
Actually, the reentrancy behavior of std::sync::Mutex is already documented:
The exact behavior on locking a mutex in the thread which already holds the lock is left unspecified. However, this function will not return on the second call (it might panic or deadlock, for example).
So std::sync::Mutex::lock is safe, and an hypothetical std::sync::lowlevel::Mutex::lock (sharing the same code btw) should have the same safe behavior.
Not exactly, as parking_lot uses a quite different implementation from std. But yes, I want the same low-level API, and as a consequence, std::sync::lowlevel::Mutex would be able to implement lock_api::RawMutex.
I've just tested it, and indeed, the two additional words carried by the guard completely changes the codegen, causing LockedList to be written on the stack and passed by pointer instead of by register. With only one additional word with std::sync::nonpoison::Mutex guard, LockedList can be passed by register (still one word passed to the function cold through the stack), but if I add another field to LockedList as it was planed before, then even nonpoison can't save me from by-pointer passing.
So the issue with the redundant reference in the mutex guard (and the poison flag) is real.
Seems like you are rolling your own mutex abstraction. There is already an established crate for that in no-std (that is commonly used in embedded at least), which is lock_api — Rust concurrency library // Lib.rs. Does that not solve your issue without reinventing the wheel?
Your proposed RawMutex also doesn't handle poisoning, which is important for reliable code, panicked threads can leave data structures in inconsistent states.
The thing @wyfo is asking for would allow an implementation of the lock_api::RawMutex trait for std’s mutex, which would be useful and is not currently possible. Right now,
If you want to do the things lock_api allows that std::sync doesn’t (e.g. ArcMutexGuard), then you also have to pick a different mutex implementation than std’s even if that’s not what you want.
If you want to be generic over mutex implementations using lock_api, std’s mutex can’t be one of the available options because it can’t implement the trait.
As @kpreid said, I rolled my own mutex abstraction precisely because lock_api can't support std::sync::Mutex. The sole purpose of my own Mutex trait is to support std::sync::Mutex, and there is of course a blanket implementation for types already implementing lock_api::RawMutex. So I didn't reinvent the wheel; I just circumvented a current limitation of the stdlib, limitation I'm asking to lift in order to be able to simply use lock_api.
Poisoning is a good default for general purpose high-level code. But I remind you that even the stdlib is working to expose a nonpoison::Mutex, so non-poisoning has an interest. And again, I'm not talking about general purpose high-level code, I'm talking about low-level unsafe panic-free code, where poisoning is just tripling the number of assembly instructions and spilling registers.
I think adding these methods to the existing Mutex is better. Besides saving a type, mixing locking kinds can be useful sometimes (and then you can use data_ptr() to access the data).
In this scheme, locking would be done by mem::forget(mutex.lock().unwrap()) (or perhaps a method for this purpose on MutexGuard, for better documentation of the usage pattern). This would be the minimal extension that would enable it to be used as a raw mutex, and avoids needing to duplicate both lock() and try_lock().
If desired, it could be constrained to only mutexes without their own data, which might help avoid confusion with how a Mutex is normally unlocked, but prevents “mixing locking kinds”:
Only the unsafe unlock method would need to be added, as you can just forget the guard returned by lock/try_lock. And then nonpoison::Mutex<()> would be enough to replace my lowlevel::Mutex proposal. However, as I wrote in the initial message, I think a dedicated type is better suited, but that's mostly a matter of taste.
My only wish is to have a lock_api::RawMutex compatible futex-based implementation without pulling an additional dependency when the stdlib already includes it.
I mentioned it, and that was in fact the first thing I wanted to propose, but then I realized that it was not really compatible with the poisoning semantic of the current std::sync::Mutex. In fact, poisoning cannot be handled by unlock alone, because it needs the value of the poison guard which is initialized while locking and stored in the mutex guard. This is what allows to not poison a mutex when you lock it in an unwinding context. So unlock would only be suited in nonpoison::Mutex, but then you have a asymmetry between nonpoison and poison APIs.
Moreover, mixing both low-level and high-level APIs into one type makes me think that it encourages mixing them in the same use case. Honestly, I can't see a case where they would be mixed, that's why I think a dedicated type is better here.