Need a spinlock that never uses std::thread::yield_now

[jimblandy]

This use case may be a little obscure, so "we don't support that" is a perfectly reasonable response. But, just in case:

On a POSIX system, I have some shared data which I need to access from multiple threads but also from a signal handler. This is generally not possible:

1. Mutex is based on pthread_mutex, which is not async-signal-safe.
2. Even if it were, if a thread holding the lock gets a signal delivered, then the handler's attempt to acquire the lock that the thread already holds is forbidden, and will cause deadlocks or panics.

I can work around 2) by simply blocking the signals whenever I hold the lock.

I was hoping to use this crate to work around 1), but the std feature means that I cannot count on this crate's spinlocks not to use std::thread::yield_now, which (oddly) is not async-signal-safe.

Even if I don't enable the std feature myself, crates used by multiple dependents are compiled with the union of the features requested by all those dependents. So if my crate happens to be used with someone else who wants spin with the std feature, then I get libc::sched_yield calls in my signal handlers.

My concerns would be addressed if there were a separate Mutex type that was guaranteed never to use sched_yield or any other non-async-signal-safe system calls, regardless of which features were enabled.

Thanks for your consideration!

[zesterer]

Hello. This is an interesting (and frustrating) product of cargo's somewhat bizarre feature flag logic. This makes me wonder whether, instead of an std feature, we should have a no_std feature and have it be the default. Are you aware of any potential problems with such an approach?

Edit: One such problem could be that an std-supporting crate might intend to use thread yielding, but then another crate pulls in spin with no_std enabled, causing priority inversion for the std-supporting crate. This is obviously undesirable too. It's almost like we need mutually-exclusive std and no_std features, but I can't imagine that working well without breaking a lot of builds that pull in spin multiple times with different features enabled. This is mentioned here.

[zesterer]

There's also some potentially relevant discussion on this thread

[zesterer]

This sounds like the same issue.

[jimblandy]

It seems to me that a client could have three possible requirements:

a) They may always need an async-signal-safe lock. (My case.)
b) They may always need an OS-friendly lock. (Currently, what people get if they request the std feature.)
c) They may have no requirements themselves, but want to be usable in both std and no-std cases. (Currently what people get if they do not request the std feature.)

I would say these are all useful and reasonable.

So perhaps what the crate should do is:

• Have a non-default std feature (as it does now).
• Unconditionally define a mutex::no_yield::SpinMutex that is async-signal-safe. Case a) would use this.
• If the std feature is enabled, define mutex::yield::SpinMutex that is not async-signal-safe but that uses OS features to yield. Case b) would request the std feature and use spin::mutex::yield::SpinMutex.
• Define mutex::general::SpinMutex as an alias for mutex::yield::SpinMutex if available, or mutex::no_yield::SpinMutex otherwise. Case c) would use mutex::general::SpinMutex

The ticket_mutex feature could also influence the definition of mutex::general::SpinMutex.

By putting all three in submodules of spin::mutex, users have to make an explicit choice, so they're unlikely to choose the general mutex by accident when they have stricter requirements. Users who don't have specific requirements and perhaps aren't informed about the issues will recognize that general as the most-trodden not-doing-anything-weird path, which is correct. And since it's all done by use statements, making the choice explicit doesn't really clutter up their code too much.

[ghost]

I'm wondering about an alternate approach that has the mutex generic over a "yield policy" (perhaps with a better name like "relax"). Something like the following:

pub trait Yield {
    fn yield();
}

This crate could provide the existing policies like AtomicYield that uses core::sync::atomic::spin_loop_hint (and conditionally ThreadYield that uses std:thread::yield_now based on the std feature), and would also allow clients to define their own yield policy for specific use cases. I also like the submodule idea above for clients that don't want to care about the general case of a custom policy.

[zesterer]

This seems like a reasonable suggestion. If anybody wants to open a PR to add this I'd be grateful. If not, I'll have the time to implement it in a few weeks.

[Ericson2314]

Yeah yield policy parameter + features for default yield policy sounds good.

[zesterer]

I've opened #102. However, it still has unresolved problems relating to type inference (more information in the PR). If anybody has any ideas about this, please put them in the PR.

[zesterer]

#102 has now been merged and included in version 0.8.0. You can select a different relax strategy using an additional type parameter. For example, spin::mutex::Mutex<T, Spin> or spin::mutex::Mutex<T, Yield>. Note that you'll need to use the fully qualified path instead of the type aliases in the root of the crate due to the inference issue mentioned in the PR.