Open RalfJung opened 5 years ago
getrandom
enables spin_no_std
feature only for UEFI targets. Looks like due to the old Cargo bug this feature gets enabled for all targets... I think we can remove spin_no_std
feature relatively easily, see the linked getrandom
issue.
Looks like due to the old Cargo bug this feature gets enabled for all targets
Yes, that's probably https://github.com/rust-lang/cargo/issues/2524.
see the linked getrandom issue.
Which one would that be?
I think we can remove spin_no_std feature relatively easily, see the linked getrandom issue.
The fact remains though that any crate making a similar mistake anywhere in the crate tree will globally enable spinlocks. That seems like a huge footgun.
Which one would that be?
I was writing it. :) It's rust-random/getrandom#50.
Yes, spin_no_std
is not strictly speaking correct. A better approach may be to use spin
by default and introduce std
feature which will be enabled by default. Although I am not sure if it will work properly in the presence of the Cargo bug, i.e. will it be possible to disable std
feature if crates supports several targets, some of which use std
and some of which don't. Also this way lazy-spin
will not be an optional dependency anymore.
So I think fixing the feature handling bug in Cargo should take priority.
I think a "proper" solution would be a locking crate that uses parking_lot when there is an OS and falls back to spinning when there is not.
That would basically be a #[no_std]
locking crate that anyone can just use without thinking about features or cfg
or so.
Thanks @RalfJung!
Yeh this sounds like a footgun, even though I'd kind of expect those wasted cycles to be amortized over the complete running time of the process. It would at least be better if we had an opt-out std
feature rather than an opt-in no_std
feature, but I don't think we could switch to that model without breaking existing users of no-std.
So the "proper" solution of an external locking crate that can determine what strategy to use opaquely sounds like our best way forward to me. I'm guessing this crate could rely on #[cfg(target)]
or something.
Cc @matklad who seems to recently be interested in spinlocks ;)
I think a "proper" solution would be a locking crate that uses parking_lot when there is an OS and falls back to spinning when there is not.
So the "proper" solution of an external locking crate that can determine what strategy to use opaquely sounds like our best way forward to me.
I'd love to push back on these two proposals.
First, #[no_std]
absolutely does not imply the absence of an OS. For example, a #[no_std]
can be used to build a static library which exposes a C ABI and is expected to be linked to from the userspace libraries. Another case is getrandom
crate, which is no_std
, but talks to the OS via libc
.
Second, even it is genuinely no-OS scenario, you can't "just" use a spinlock and call it a day, at least because there are two kinds of spinlocks you may use in on bare metal (saving/notsaving interrupts), and it is wrong for the library to pick any particular one: https://www.kernel.org/doc/Documentation/locking/spinlocks.txt.
That makes sense. Do you have any alternative proposals though? What you say sounds a lot like "#[no_std]
can't do locking (or any other kind of blocking concurrency)". If so, should lazy-static remove #[no_std]
support?
What you say sounds a lot like "#[no_std] can't do locking (or any other kind of blocking concurrency)"
That's my opinion, yeah. My hypothesis is that most of the uses of lazy_static
in no-std scenarios could and should be replaced by "racy" atomic initialization (when potentially more than one thread computes the result, but only one stores it) (like in std-detect), or using appropriate blocking primitives (like in getrandom), or changing the interface so that there's an explicit "init" function which produces an evidence of initialization, which is then threaded through the rest of the API
If so, should lazy-static remove #[no_std] support?
In the ideal world, yes. In the real world, the API is published and stable, and I think removing it will cause more churn than potential benefits. Hopefully, the problem will solve itself once we agree on, implement and stabilize standard lazy type.
I have a tiny crate (really just a crate to test miri) where I noticed that after a
cargo update
,lazy_static
depends onspin
. Or, at least,spin
gets compiled. (If you want to reproduce, get that folder and docargo update
; I couldn't land the updated version yet due to other problems.)Isn't that a problem? The fact that
spin_no_std
is a feature means that if any create enables it, then globally all lazy statics will use a spin lock. Given that there is no way to know what kind of computation is done in the initializer, spinning instead of properly putting waiting threads to sleep seems like a huge waste of CPU cycles.I am not entirely sure what exactly is enabling the feature in my case, the reverse dependency graph of
spin
is not very big:@Mark-Simulacrum on Zulip suggested that Cargo ignores
cfg
restrictions for feature flags, and thusgetrandom
is likely the trigger here. Cc @newpavlovFWIW, the fact that there is a negated term ("no") in the feature also indicates this is probably a bad approach, see the feature name guidelines. Cargo features are additive; creates can only ask for a feature to be enabled but never ask for a feature to be disabled.