Add an atomic-polyfill optional dependancy

[ogoffart]

To make it compile on platform that have limited atomics

Fixes #155

Note that this re-implement the Debug version of the OnceBox because atomic-polyfill::AtomicPtr don't implement Debug (https://github.com/embassy-rs/atomic-polyfill/issues/11)

[ogoffart]

Ping?

[matklad]

Sorry, didn't get the chance to look into this closely.

On the first blush, I am unsure about this -- I am not a huge fan of the solutions in the style of "silently replace mutexes with spinlocks" -- I generally thing the caller should be responsible for selecting the right synchronization primitive.

This case looks like it might actually be OK though? What this does is disabling interrupts for the duration of the CAS, and that should be OK, as we are not calling any user-supplied code within the scope of critical section? What happens if there are multiple cores/CPUs in play)? Or are we sure that that that's not a problem for these targets?

I guess, given that this is a disabled by default features, it's not that important. Could you bump a version in Cargo.toml as well, such that this gets a release automatically when merged?

[ogoffart]

The use-case is indeed to be able to make cross platform libraries that use once_cell::race so they can be used on both platform with a std library, and also on bare metal embedded on device that do not have threads and atomic.

I am not a huge fan of the solutions in the style of "silently replace mutexes with spinlocks" -- I generally thing the caller should be responsible for selecting the right synchronization primitive.

I have read https://matklad.github.io/2020/01/02/spinlocks-considered-harmful.html and I agree 100% But in this case, there is probably no spinlock involved, even when this feature is enabled If this is a platform that supports atomic, then the core::sync::atomic classes are used so no change at all. If the platform does not support atomic, it depends on how the critical_section crates implements it. In single core scenario, this is just about disabling the interrupt for the few instructions. On multi core device, there might be a spinlock. but how else could we implement it?

What happens if there are multiple cores/CPUs in play)? Or are we sure that that that's not a problem for these targets?

The atomic-polyfill README states: https://github.com/embassy-rs/atomic-polyfill

Note: polyfill is based on critical sections using the critical-section crate. The default implementation is based on disabling all interrupts, so it's unsound on multi-core targets. It is possible to supply a custom critical section implementation, check the critical-section docs for details.

Could you bump a version in Cargo.toml as well, such that this gets a release automatically when merged?

Should it be 1.8.1 or 1.9 ?

[matklad]

Should it be 1.8.1 or 1.9 ?

1.9

so it's unsound on multi-core targets

Yeah, it is good to know. Let's add some scary-sounding warning to the feature in Cargo.toml, something like "this can be unsound. Please read docs of atomic-polyfill (link) and make sure you understand all the implications".

[matklad]

I guess, my initial knee-jerk reaction was overly negative. I would push back strongly against this being enabled by default, but having sketchy things under a feature flag is totally fine :-)

[matklad]

bors r+

Thanks!

[bors[bot]]

Build succeeded:

• Rust