Silent failure of std::sync::Once when in const variable

[CGamesPlay]

I tried this code playground link:

use std::sync::Once;

const INIT: Once = Once::new();

fn main() {
    INIT.call_once(|| {
        println!("Global Once::call_once first call");
    });
    INIT.call_once(|| {
        println!("Global Once::call_once second call");
    });
}

The problem is that my Once is in a const and not a static. However, I would expect a compile error or a runtime error instead of broken behavior here. The same behavior is exhibited in std::sync::LazyLock and probably std::sync::OnceLock, due to using a Once internally.

Thanks for considering this!

Meta

Checked on 1.82.0 and on nightly.

[juntyr]

This code is behaving correctly but unintuitively. A const value behaves "as-if" it is copied in at every usage point, so each call gets a fresh Once to work with.

I agree that this is a potential papercut. Ideally, there would be a clippy lint to detect any std::sync or std::cell type in a const, since it a static may have been intended instead.

[yingmanwumen]

Ideally, there would be a clippy lint to detect any std::sync or std::cell type in a const...

@juntyr
There has been a clippy rule named 'declare_interior_mutable_const' that would give a warning in this situation: a const item should not be interior mutable, consider making this a static item.
However, the compiler itself doesn't deal with it for it doesn't break any basic grammars or semantic rules... Though I agree that a const sync type item is counter-intuitive and might behave unexpectly, it still worths considering that if the compiler should capture it as a compiling warning/error. @CGamesPlay

[juntyr]

There has been a clippy rule named 'declare_interior_mutable_const' that would give a warning in this situation: a const item should not be interior mutable, consider making this a static item.

That’s great, thank you for pointing out that it already exists!

[CGamesPlay]

Didn't know about the clippy lint, it definitely would have immediately indicated this problem to me.

So, is the fact that LazyLock accepts an FnOnce which is called more than one time not a safety issue? I honestly don't know, but in any case it's a lie to the library consumer.

[juntyr]

So, is the fact that LazyLock accepts an FnOnce which is called more than one time not a safety issue? I honestly don't know, but in any case it's a lie to the library consumer.

The FnOnce is technically only called once. The issue is that because you are using a const, every time you name it (in your two calls), it’s as if you had pasted the const initialisation code to this usage to construct a fresh LazyLock with a fresh FnOnce that is then only called this once. On the next usage of the const, you are referring to a completely unrelated LazyLock with a completely unrelated FnOnce.

Again, this is unintuitive but the fully correct behaviour of consts, since they by definition have no state that can change at runtime.

Perhaps the docs of these types could have an extra warning to call out this papercut specifically?

[CGamesPlay]

Yeah... it definitely feels bad. The explanation makes sense, and leaves me with the question, "why would anyone want to do this?"

It seems to me that anyone using LazyLock, Once, etc in a const is very likely creating a bug. Perhaps a solution would be to add an attribute like must_use, but for non_const? Then applying that attribute to LazyLock would result in a compiler warning when a LazyLock is assigned to a const.

[append]
For reference, clippy's borrow_interior_mutable_const is certainly useful here, but is warn by default, presumably because it can't be 100% certain that the issue is a bug (see "known issues"). So annotating the type may be necessary.

[juntyr]

Yeah... it definitely feels bad. The explanation makes sense, and leaves me with the question, "why would anyone want to do this?"

It seems to me that anyone using LazyLock, Once, etc in a const is very likely creating a bug. Perhaps a solution would be to add an attribute like must_use, but for non_const? Then applying that attribute to LazyLock would result in a compiler warning when a LazyLock is assigned to a const.

Agreed. I’ve personally used them in consts but only for type level trickery where I was absolutely sure of what I was doing. So making the lint a bit more on the nose (e.g. by up streaming it from clippy to rustc) would likely avoid many logic bugs while not being too annoying in the few cases where we do need locks and cells in consts.

[asquared31415]

The rustc warn by default const_item_mutation lint might be useful to extend to trigger on all types that may contain interior mutability? This would be a problem for Cell and other interior mutability constructs too.

[cyqsimon]

Ran into this today.

I had a const ITEM: T = const_fn(); which I had to refactor to call a non-const function. So I threw in a LazyLock<T> but forgot to change const to static. Was very surprised when non_const_fn flooded my terminal with repeated logs when I expected it to run only once.

Had I not had logging in non_const_fn, this is something I would have missed entirely. Or at least until the next time I run clippy. Fortunate for me that in this case it would only be a performance hit and not (yet) a logic bug. So yeah, I think this should definitely be at least a warning in rustc.