Creating a static, shared reference to editable values is unsound

[HeroicKatora]

The comment above this line is inaccurate as to what it is doing, and the usage of transmute here to create a reference with longer lifetime is not sound.

const-tweaker/macro/src/lib.rs

Lines 118 to 120 in fd94d62

	// Make the reference static, so it leaks, but that shouldn't matter
	// because there will always be one reference since the dashmap is global
	std::mem::transmute::<&#ty, &'static #ty>(value)

The transmute itself does not cause a leak of the value. It is purely a pointer cast, and has no semantic effect on the machine state. What this is doing is the opposite, it is asserting to the compiler that your code has done something to the effect of leaking the value, but that the compiler just can't see. That, however, is wrong.

Also note that creating a statically shared reference to the value is not what you want as that would disallow any mutable reference being created for the whole remaining program lifetime. Creating any mutable reference is UB as long as a shared reference exists, thus it would be allowed for the compiler to optimize out any of the writes done by the web server. More likely though, it will instead deduplicate some reads of the referred-to value which could cause writes to appear to have no effect.

[tversteeg]

The comment above this line is inaccurate as to what it is doing, and the usage of transmute here to create a reference with longer lifetime is not sound.

const-tweaker/macro/src/lib.rs

Lines 118 to 120 in fd94d62

	// Make the reference static, so it leaks, but that shouldn't matter
	// because there will always be one reference since the dashmap is global
	std::mem::transmute::<&#ty, &'static #ty>(value)

The transmute itself does not cause a leak of the value. It is purely a pointer cast, and has no semantic effect on the machine state. What this is doing is the opposite, it is asserting to the compiler that your code has done something to the effect of leaking the value, but that the compiler just can't see. That, however, is wrong.

I see I didn't properly understood what it does, it's the first time I've experimented with unsafe code. I'm wondering why it's wrong though?

Also note that creating a statically shared reference to the value is not what you want as that would disallow any mutable reference being created for the whole remaining program lifetime. Creating any mutable reference is UB as long as a shared reference exists, thus it would be allowed for the compiler to optimize out any of the writes done by the web server. More likely though, it will instead deduplicate some reads of the referred-to value which could cause writes to appear to have no effect.

I guess I see the problem with this, but then I'm wondering why it's working in its current state? And what would you suggest would be a better way to implement this?

Thanks for the issue/introspection!

[HeroicKatora]

I'm wondering why it's wrong though?

If you use unsafe then you should wonder and explain why it is right, since this is effectively a promise to the compiler. If you can't uphold that promise by fact then the use of unsafe is likely incorrect.

I guess I see the problem with this, but then I'm wondering why it's working in its current state? And what would you suggest would be a better way to implement this?

The compiler does not have to any particular thing once UB occurs, it's allowed to do 'what you expect' or other diverging behaviour. That's why it is called undefined behaviour. It may at some point in the future give very hard to debug problems. In this case, I do have some sample program to demonstrate strange effects right away:

// Create a slider to tweak 'VALUE' in the web GUI
#[const_tweaker::tweak]
const VALUE: f64 = 0.0;

fn main() {
    // Initialize the web GUI at 'http://127.0.0.1:9938'
    const_tweaker::run().expect("Could not run server");

    let value: &'static _ = &*VALUE;
    // Enter a GUI/Game loop
    loop {
         assert_eq!(*value, 0.0);
    }
}

If run in release mode, this assert never triggers on my machine even when changing the value through the web interface. That's because the static reference given by this library asserts to the compiler that the value will never change. The compiler, wanting to avoid wrok, may then only check the condition in the first loop iteration and subsequently the program may simply enter an unchecked infinite loop.

As said, the compiler will allow but not obligated to perform this kind of inlining. This will come back to bite easily when in any larger production different choices are made for different parts of the program, which the compiler is perfectly allowed to do because your promises do not hold. Then suddenly there are diverging view of the single constant that may cause all sorts of following misbehaviour.

[tversteeg]

I think I understand it now, but I still don't know how to fix that.

The problem I tried to solve by using the unsafe transmute is that the std::ops::Deref on the generated struct always has to return a reference. The result of that is that I need to get the lifetime of the value from the map, which I can't because I can't control where the dereference happens and with which lifetime.

I tried to use the transmute here because I know that the value of the maps will ever be removed and I assumed that signifying that with a 'static lifetime was enough. I didn't know the compiler was smart enough to only check on the first loop iteration.

[a1phyr]

I had quite the same issue with assets_manager: I wanted users to keep references to assets while having hot-reloading running in the background.

I came up with a solution involving a RwLock and interior mutability. However, due to lifetime requirements, users have to lock it manually whenever they access it. This may not be what you want, though.

Given that you only support "basic" types, maybe atomic types can help you. It won't work for &str easily (maybe something can be done with Arc or AtomicPtr and some unsafe code), but it can be a solution for all others.

[zesterer]

This is not great and is likely to be an actual soundness bug that affects real-world code. The DashMap will reallocate as it grows, leaving a dangling pointer in place after this transmutation.

My advice: Put the Field inside a Box, then Box::leak it to safely have the Field become 'static. Alternatively, you could use Arc and then Arc::make_mut when you want to mutate the constant via the web GUI.