Keeping references to #[thread_local] statics is allowed across yields.

[eddyb]

This does not affect stabilization of async fn unless #[thread_local] is also stabilized

Try on playground:

#![feature(generators, generator_trait, thread_local)]

use std::ops::Generator;
use std::sync::atomic::{AtomicUsize, Ordering};
use std::thread;

#[thread_local]
static FOO: AtomicUsize = AtomicUsize::new(0);

fn foo() -> impl Generator<Yield = (String, usize), Return = ()> {
    static || {
        let x = &FOO;
        loop {
            let s = format!("{:p}", x);
            yield (s, x.fetch_add(1, Ordering::SeqCst));
        }
    }
}

fn main() {
    unsafe {
        let mut g = thread::spawn(|| {
            let mut g = foo();
            println!("&FOO = {:p}; resume() = {:?}", &FOO, g.resume());
            g
        }).join().unwrap();
        println!("&FOO = {:p}; resume() = {:?}", &FOO, g.resume());
        thread::spawn(move || {
            println!("&FOO = {:p}; resume() = {:?}", &FOO, g.resume());
        }).join().unwrap();
    }
}

Sample output:

&FOO = 0x7f48f9bff688; resume() = Yielded(("0x7f48f9bff688", 0))
&FOO = 0x7f48fafd37c8; resume() = Yielded(("0x7f48f9bff688", 1))
&FOO = 0x7f48f9bff688; resume() = Yielded(("0x7f48f9bff688", 0))

You can see by the pointer addresses and values inside FOO that the same location was reused for the second child thread (it's a bit harder to show a crash) - this is clearly an use-after-free.
If we had in-language async, the same problem could be demonstrated using those.

In non-generator functions, such references have function-local lifetimes and cannot escape.
With the stable thread_local! from libstd, user code gets access to the reference in a (non-generator/async) closure, which also doesn't allow escaping the reference.

cc @alexcrichton @withoutboats @Zoxc

[comex]

Am I right in assuming the same issue applies to implementations of thread locals in external crates, which aren't limited to unstable? e.g. https://amanieu.github.io/thread_local-rs/thread_local/index.html

edit: I'm wrong. From that page:

Per-thread objects are not destroyed when a thread exits. Instead, objects are only destroyed when the ThreadLocal containing them is destroyed.

Otherwise the API would be blatantly unsound even without generators.

edit2: ignore me; I misunderstood the scope of the problem. For the record, it doesn't apply to thread_local!, or anything else that uses a callback.

[nikomatsakis]

triage: P-medium

Should fix, not urgent.

[bstrie]

If we had in-language async, the same problem could be demonstrated using those.

Now that async/await is stable, can anyone reproduce this bug using only those? I gave it a whack (playground here) and got the following error message:

error[E0712]: thread-local variable borrowed past end of function
  --> src/main.rs:10:5
   |
10 |     &FOO
   |     ^^^^ thread-local variables cannot be borrowed beyond the end of the function
11 | }
   | - end of enclosing function is here

...which suggests at least some degree of mitigation is already implemented, but it's possible that my translation (which obviously can't yield directly) did not represent the spirit of the original bug.