Static thread_local! declarations are moved before Drop is called

[orlp]

Consider this piece of code:

use std::sync::atomic::*;

static UNIQ_ID: AtomicU64 = AtomicU64::new(0);

struct Foo(u64);

impl Foo {
    fn new() -> Foo {
        Foo(UNIQ_ID.fetch_add(1, Ordering::Relaxed))
    }
}

impl Drop for Foo {
    fn drop(&mut self) {
        eprintln!("{} dropped with addr {:p}", self.0, self);
    }
}

fn main() {
    thread_local!(static FOO: Foo = Foo::new());
    FOO.with(|foo| {
        eprintln!("{} living with addr {:p}", foo.0, foo);
    });
}

I would expect the statically declared FOO to not move around after initialization. In fact, Foo might be a type which may not be moved around after initialization at all, such as a type containing pthread_mutex_t. However, we see that it does get moved before the drop:

0 living with addr 0x6000002000a8
0 dropped with addr 0x16f9faaa8

I believe the issue lies here:

rust/library/std/src/sys/thread_local/native/lazy.rs

Lines 95 to 98 in e964cca

	// Update the state before running the destructor as it may attempt to
	// access the variable.
	let val = unsafe { storage.state.get().replace(State::Destroyed(())) };
	drop(val);

This code should be changed to not use an enum but rather a separate state flag + cell, so that the value is not moved.

[Amanieu]

cc @joboet

[joboet]

std doesn't promise immovability for thread locals anywhere, so this isn't a bug. Still, I think asking for such a promise isn't totally unreasonable. When I rewrote the TLS implementation, I thought that the niche optimisation made possible by using an enum would be more valuable than immovability, especially since I still think that it wouldn't be possible to use LocalKey together with Pin – but I don't feel strongly.

[Amanieu]

The specific issue that triggered this is that parking_lot keeps a pthread_mutex_t in a thread-local which cannot be moved after it has been initialized (miri checks and enforces this).

[kpreid]

When I rewrote the TLS implementation, I thought that the niche optimisation made possible by using an enum would be more valuable than immovability, …

An argument for immovability:

The comparison that comes to mind here is that for every instance of a thread-local variable, there is a thread, and threads have stacks, which in most programs have to be generously oversized for reliability, and adding a single byte discriminant seems insignificant compared to that. Is thread-local storage space especially limited in some way, such that that byte is more valuable here?

An argument for the status quo:

That byte is a cost to all thread-locals, and applications that need the value not to move can use Box<T> (or even Pin<Box<T>>) instead.

[orlp]

@kpreid Sorry my previous claim we could get the best of both worlds doesn't work. I think just paying the 1 byte per thread-local penalty is the best solution.

Then again, the 1-byte penalty might not actually be 1 byte after alignment.