lazy_static uses spinlocks in my with-std crate

[RalfJung]

I have a tiny crate (really just a crate to test miri) where I noticed that after a cargo update, lazy_static depends on spin. Or, at least, spin gets compiled. (If you want to reproduce, get that folder and do cargo 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:

spin v0.5.0
└── lazy_static v1.3.0
    ├── c2-chacha v0.2.2
    │   └── rand_chacha v0.2.0
    │       └── rand v0.7.0
    │           [dev-dependencies]
    │           └── cargo-miri-test v0.1.0 (/home/r/src/rust/miri/test-cargo-miri)
    └── getrandom v0.1.5
        ├── rand v0.7.0 (*)
        └── rand_core v0.5.0
            ├── rand v0.7.0 (*)
            ├── rand_chacha v0.2.0 (*)
            ├── rand_hc v0.2.0
            │   [dev-dependencies]
            │   └── rand v0.7.0 (*)
            └── rand_pcg v0.2.0
                └── rand v0.7.0 (*)
                [dev-dependencies]
                └── rand v0.7.0 (*)

@Mark-Simulacrum on Zulip suggested that Cargo ignores cfg restrictions for feature flags, and thus getrandom is likely the trigger here. Cc @newpavlov

FWIW, 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.

[newpavlov]

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.

[RalfJung]

Looks like due to the old Cargo bug this feature gets enabled for all targets

Yes, that's probably rust-lang/cargo#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.

[newpavlov]

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.

[RalfJung]

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.

[KodrAus]

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.

[RalfJung]

Cc @matklad who seems to recently be interested in spinlocks ;)

[matklad]

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.

[RalfJung]

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?

[matklad]

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.