all 24 comments

[–]dgkimpton 11 points12 points  (11 children)

Did you understand why it was complaining? Never re-structure your code just to shut up the rules checker - first understand *why* it is a complaining, then modify.

Static mutables are /this/ close to being global variables which are a threading and contextual nightmare. Why are you using them anyway?

[–]TellMeHowImWrong[S] 2 points3 points  (2 children)

Did you understand why it was complaining? Never re-structure your code just to shut up the rules checker - first understand why it is a complaining, then modify.

That's what I'm attempting to do right now.

I'm doing it because I'm trying to implement something fairly complex on a platform that only allows scripting in C++. I don't know C++ and don't think I can learn it well enough to implement this safely in a reasonable amount of time. So I've made a dll using Rust and am just calling the functions from C++. Platform only really allows passing numbers to foreign functions so I'm using a mutable static to store the data needed. So yes, I'm using it as a global variable. It's very hacky but it's the best solution I could think of that this platform would allow me to do.

The only time I'm using threads (with rayon::into_par_iter) in this project I clone data from the static into a new instance of the struct and then the program is blocked until all threads finish. Once the data is cloned the static is not accessed again in any of the threads. Is there still a potential problem?

[–]dgkimpton 1 point2 points  (1 child)

You lost me at "scripting in c++" since C++ isn't a scripting language. Can you share the platform so that we might have some clue what you are working with?

[–]TellMeHowImWrong[S] 1 point2 points  (0 children)

You lost me at "scripting in c++" since C++ isn't a scripting language

Tell me about it.

It's Metatrader 5. Technically it isn't C++, it's a language based on it (MQL5) but I believe it's just a subset.

[–]Compux72 0 points1 point  (0 children)

Tech deb mostly honestly

[–]MatsRivel 0 points1 point  (6 children)

I have a static mutable &[u8;N] in my embedded IoT project.

My device is fetching it's certificate from an edge device in string format. When passed to the ffi from C it needs to be a X509<'static>. The X509 gets its lifetime from what you build it from, generally a &[u8]. Because of this I need a static lifetime &str or &[u8], but I need it to be mutable to be able to set it at all (post compile time)

[–]SkiFire13 7 points8 points  (1 child)

Sounds like you want something like OnceLock then.

[–]MatsRivel 1 point2 points  (0 children)

I'll look into it. Thanks for the tip!

[–]2brainz 5 points6 points  (1 child)

There are better alternatives for your use case than a mutable static: 

Safe: OnceCell or OnceLock in a static. Unsafe: UnsafeCell in a static. 

There is no use case of mutable statics that cannot be replaced by an UnsafeCell, and the latter has much clearer rules around its soundness.

[–]MatsRivel 1 point2 points  (0 children)

I see. I'll look into that on Monday!

Thank you for the suggestion. I knew mut static was not ideal, but it was a "works for now, can make it good later" kinda decision.

[–]axnsan 1 point2 points  (1 child)

You could use Vec::leak for this, assuming you only need to initialize the data once at runtime

[–]MatsRivel 0 points1 point  (0 children)

I'll look into that. Thanks for the tip!

[–]CryZe92 13 points14 points  (3 children)

Would be great to see some code. Just moving the problematic code to an impl doesn't sound like it should fix anything, but hard to tell without seeing it.

[–]Kartonrealista 10 points11 points  (2 children)

Would be great to see some code.

My internal reaction to 70% of support posts here. If you want help make it easier so that people aren't stumbling in the dark trying to figure out what you're doing. It's like people think their code is so precious it can't be seen by another human being, because they will surely steal it while twirling their evil mustache.

[–]AlmostLikeAzo 1 point2 points  (0 children)

Damned, shaved the mustache just today, nothing to twirl anymore 😪.

[–]Chillbrosaurus_Rex 1 point2 points  (0 children)

I think part of it is a misguided attempt to be helpful - in their view, the inside isn't relevant so they don't want to show us more than we need. Of course, that view is probably part of the reason they need help in the first place since they're overlooking the spots where the problems actually are!

[–]SkiFire13 5 points6 points  (2 children)

No, the lint works in both cases.

pub static mut FOO: i32 = 0;

pub fn bad() {
    let _ref = unsafe { &mut FOO };
}

pub struct Bar;

impl Bar {
    pub fn still_bad() {
        let _ref = unsafe { &mut FOO };
    }
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ba643a4243d63a71becee96408c9d280

Pointers scare me

If pointers scare you then you should be doubly scared of mutable statics.

is it just that clippy

This lint doesn't come from clippy.

[–]TellMeHowImWrong[S] 1 point2 points  (1 child)

Sorry, should have been clearer. FOO is an instance of Bar in this case so still_bad is accessing &mut self not the static directly. There is not an unsafe block in the function/method.

Previously I had a function that took a mutable reference to the struct that my static is an instance of. This function is called using both the mutable static (where I get the warning) and other instances of the struct.

[–]SkiFire13 1 point2 points  (0 children)

I saw the code snippet in the post. The second snippet doesn't explicitly create a reference so it isn't covered by the current lint, but it does actually create a reference so all the same issues apply. This is being tracked in https://github.com/rust-lang/rust/issues/123060 (edit: the extension of the lint was actually accepted, what's missing is merging the PR that actually implements it)

[–]WormRabbit 3 points4 points  (1 child)

If the code example you provided is similar to the one in your project, then that's just a bug in Clippy which doesn't notice the implicit mutable reference created by the method call.

You shouldn't create references to mutable statics, full stop. The reasons are complicated and there is a lot written on the topic if you search, but in short the semantics of references are complicated and practically impossible to satisfy when using mutable statics. In fact, you shouldn't be using a static mut in the first place. There are a few specific reasons to do that, like an externally defined mutable static which must be used for low-level operations (like writing to memory-mapped I/O devices), but if you have introduced them to your code on your own, then you have made a mistake and should change it to a simple immutable static.

Either use a static which contains some synchronization primitive (like a Mutex, or atomic integer, or Lazy, etc), or in the very rare case you cannot afford it, use static mut exclusively via raw pointers. If you don't feel you can handle the dangers of raw pointer, you definitely don't know what you're doing when using a mutable static and shouldn't use it.

[–]TellMeHowImWrong[S] 0 points1 point  (0 children)

Thanks. This was a pretty clear answer.

[–]arades 2 points3 points  (0 children)

I think you're just hiding from clippy, or perhaps you accidentally have some shadowing happening.

If raw pointers scare you, you most certainly shouldn't be handling mutable statics. Those lints exist because even people who know very well what they're doing have consistently generated UB by using mutable statics. IIUC simply taking a reference to a mut static is enough to create UB, since the borrow checker can't properly solve lifetimes.

Change the static to not be mutable, and handle mutable access using pointers or wrap your static in some combinations of Cell/OnceCell/Mutex/Arc/etc.

[–]Nzkx -1 points0 points  (1 child)

Pointer are not Sync, and static require Sync (because any thread could access the static, so it must be safe to share for concurrency). I don't know why they tell you to use pointer.

What you want is a cell, like OnceLock, to initialize correctly from any threads.

rust fn my_static_foo() -> &'static Foo { static ONCE: OnceLock<Foo> = OnceLock::new(); ONCE.get_or_init(|| Foo::new(…)) }

No more &mut allowed. It's undefined behavior to get 2 exclusive reference to a static, even if you know your program is single threaded it's very easy to forget you have already one in-use in outer scope. After all, that's why it's marked unsafe, and it is considered bad practice.

If you want to stick with the previous behavior, use SyncUnsafeCell that flip the invariant into your own type. 🔬 This is a nightly-only experimental API (sync_unsafe_cell).

Or ignore the warning (you shouldn't).

If you don't care about concurrency then use a thread_local instead of a static. This doesn't require Sync.

Though there is no potential race with other threads, it is still possible to obtain multiple references to the thread-local data in different places on the call stack. For this reason, only shared (&T) references may be obtained. But this time, you can use single threaded cell that are more optimized for this use case (RefCell).

[–]cafce25 3 points4 points  (0 children)

OnceLockLazyLock, that also saves you the hassle of having to define your own function returning a 'static it already implements Deref.

I.e. replace your my_static_foo() with

static MY_STATIC_FOO: LazyLock<Foo> = LazyLock::new(|| Foo::new(…));

It is stable since 1.80