std::thread::local internals allow race conditions in safe but unstable code.

[eddyb]

(try on playpen)

#![feature(const_fn, drop_types_in_const, thread_local_internals)]

type Foo = std::cell::RefCell<String>;

static __KEY: std::thread::__FastLocalKeyInner<Foo> =
    std::thread::__FastLocalKeyInner::new();
        
fn __getit() -> std::option::Option<
    &'static std::cell::UnsafeCell<
        std::option::Option<Foo>>>
{
    __KEY.get()
}

static FOO: std::thread::LocalKey<Foo> =
    std::thread::LocalKey::new(__getit, Default::default);

fn main() {
    FOO.with(|foo| println!("{}", foo.borrow()));
    std::thread::spawn(|| {
        FOO.with(|foo| *foo.borrow_mut() += "foo");
    }).join().unwrap();
    FOO.with(|foo| println!("{}", foo.borrow()));
}

Discovered while working on a sound fix for #17954. The problem is this decision which prevents the thread_local! macro from using unsafe to state that it actually has a #[thread_local] static to use in LocalKey. Without that, anything of 'static lifetime (which is btw quite wrong for #[thread_local]) and the right type can be used, from safe code.

While we can change the APIs all we want, going back to the correct solution of #[allow(unsafe_code)] could be a breaking change, without unsafety hygiene.

cc @rust-lang/libs

[eddyb]

cc @nikomatsakis @arielb1 @Manishearth

[Manishearth]

We can probably introduce a way for #[allow] to punch through forbid the same way allow_internal_unstable does. But it might be tricky.

[eddyb]

@Manishearth Exactly my thoughts but it'd be nice if we had unsafe hygiene for this.
However, if #[allow_internal_unsafe] is fine for this, it would not be hard to implement.

[Manishearth]

A slight hole in that would be that now user-written unsafe code in thread_local!() won't be caught by the lint.

....... I'm not sure what to do about that. deny(unsafe)/forbid(unsafe) is something I'd rather keep as sound as possible (yes, there are ways to circumvent it, but assuming a "non-malicious but non-smart" actor -- i.e. all of us programmers writing terrible code -- it's nice when these things are perfect)

[eddyb]

@Manishearth #[allow_internal_unstable] already doesn't have that issue, it's hygienic.

[Manishearth]

Oh, right!

[eddyb]

@arielb1 suggested using item hygiene from the new macros. So I added #[feature(macro_decl)] to libstd, changed __thread_local_inner to be a pub macro item and I got 275 missing stability, documentation, etc. errors, because private items now "appear to be" macro-accessible.
It's reproducible with just pub macro foo() {} in src/libstd/lib.rs.

So #[allow_internal_unstable] remains the only way forward for now.

[est31]

Without that, anything of 'static lifetime (which is btw quite wrong for #[thread_local])

rust-lang/rfcs#1705

[eddyb]

@est31 'thread is unsound/uncheckable. See sound fix at #43746.

[joboet]

The regression test for this issue caused build failures for me in #116123. Instead of trying to rework it to fit the new API, is everyone involved here fine with me removing it? IIUC, it only tests unstable, internal API of std, which is being reworked as part of #110897 anyway.