Need a spinlock that never uses std::thread::yield_now about spin-rs HOT 9 CLOSED

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.

from spin-rs.

There's also some potentially relevant discussion on this thread

from spin-rs.

This sounds like the same issue.

from spin-rs.

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.

from spin-rs.

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.

from spin-rs.

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.

from spin-rs.

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

from spin-rs.

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.

from spin-rs.

#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.

from spin-rs.