lazy_static "spin_no_std" feature is non-additive and can break unrelated code.

[eric-seppanen]

num-bigint-dig depends on lazy_static with features = ["spin_no_std"]. This can break other crates in the same workspace.

lazy_static's spin_no_std feature is non-additive; it causes lazy_static to replace use of std::sync::Once with spin:once::Once. This may seem like a harmless replacement, but:

• spin::once::Once<T> has different trait bounds. Namely, it only implements Sync where T: Send + Sync while lazy_static using std::sync::Once only requires T: Sync.
• This means that anyone using lazy_static on a non-Send type will see their code break if the spin_no_std feature is enabled.
• Adding num-bigint-dig as a dependency to a large workspace means that every crate in that workspace now gets the modified lazy_static code using spin with stricter trait bounds.

This is currently happening to me: I added rsa to a large workspace, and that change causes compile errors in unrelated (previously working) code:

error[E0277]: `*const u8` cannot be sent between threads safely
    --> image.rs:25:1
     |
25   | / lazy_static::lazy_static! {
26   | |     pub(crate) static ref HELLO_IMAGE: Option<ImageBuffer> = {
27   | |         let image_bytes = std::fs::read("hello.png").ok()?;
28   | |
...    |
36   | |     };
37   | | }
     | |_^ `*const u8` cannot be sent between threads safely

I'm not sure how to handle this, but it would be nice if there were a feature in num-bigint-dig to disable this behavior. As it is, I'm unable to add an rsa dependency unless I fork+patch num-bigint-dig to disable that feature.

[tarcieri]

There's not a lot of uses of lazy_static. Maybe it could be migrated to once_cell now that it's been merged into core/std?

[dignifiedquire]

I like the idea switching to once_cell. I would just use the library version for now though, as the std version was only stabilized in 1.70

[dignifiedquire]

Once servo/rust-smallvec#301 is solved, we could get rid of it entirely

[eric-seppanen]

Just to clarify, the trait bounds problem is not that significant; the more serious problem is that it forces everything in the workspace to use spinlocks instead of OS-level blocking for all lazy_static instances.

To paraphrase matklad, replacing an OS-level mutex with a spinlock is bad. If people want spinlocks for bare-metal environments, they should opt in to that approach (presumably by disabling default features). And this should not happen (by accident or omission) on targets with OS mutex support.

[tarcieri]

Perhaps we could start by disabling the spin_no_std feature?

[eric-seppanen]

Once servo/rust-smallvec#301 is solved, we could get rid of it entirely

That issue is resolved; does that mean we can fix this?

[zeerooth]

This has really bugged me for the better part of the day. Is there a way to patch this dependency at the moment?