Tracking issue for cleaning up std's thread_local implementation details

[m-ou-se]

std::thread_local, std::thread::local, std::thread::local_impl, std::sys_common::thread_local_key, std::sys_common::thread_local_dtor, std::sys::thread_local_key, etc. etc. are all messy and form quite a confusing maze. Just look at this map I tried to draw of it all:

AAAaaaaAaa

Time to clean it up.

And maybe also fix some bugs while we're at it. ^^

---

• Initial name and module cleanup without touching implementation: Restructure and rename std thread_local internals to make it less of a maze #110861
• Remove unused Key type: Remove unused std::sys_common::thread_local_key::Key #110898
• Fix panic-in-drop handling
  • A thread_local! with a const initializer and a panicking drop impl unwinds through an extern "C" frame #112285
    • Avoid unwind across extern "C" in thread_local::fast_local #112292
• Stop calling everything Key, because most of those things are actually not keys, and drop LazyKeyInner
  • Rewrite native thread-local storage #116123
  • Simplify key-based thread locals #122494
  • Rewrite TLS on platforms without threads #123724
• Reduce the amount of code in the thread_local!{} expansion. Make TLS accessors closures that return pointers #125525
• Use *const T rather than &'static T when it's not really 'static: Make TLS accessors closures that return pointers #125525
• Clean up implementation more
  • std: refactor the TLS implementation #126523
  • std: separate TLS key creation from TLS access #126953
  • std: use duplicate thread local state in tests #128135
• Store thread ID and stack guard range in thread locals without destructors (on platforms with #[thread_local])?
  • Stack guard range: Refactor stack overflow handling #123265
  • thread ID: Use ThreadId instead of TLS-address in ReentrantLock #124881
• Make thread::current thread local more efficient and make it available in (most) TLS destructors: std: make thread::current available in all thread_local! destructors #127912
• Detect stack overflow in TLS destructors: std: detect stack overflows in TLS destructors on UNIX #131282

Related issues to solve, maybe:

• Consider documenting that (parts of?) stdlib must not be used before/after main #110708
• Stack overflow not caught in Drop for TLS data #111272
  • Stack overflow in thread local's drop rendered as a segmentation fault. #109785
• Maybe this shouldn't work? See conversation around this comment.
• ...?

[joboet]

LazyKeyInner unfortunately can't just be OnceCell because of reentrant initialization: Because OnceCell::get_or_init returns a shared reference to the data that lives for the duration of self, it cannot change the value after the first inner initialization. LazyKeyInner can do that (and does) because references cannot outlive LocalKey::with.

[m-ou-se]

@joboet In what situation would that mem::replace call replace a Some instead of a None? Is that the right behaviour? Do we have a test for that situation?

[joboet]

It's very cursed, but this works. I don't think it's currently tested, however.

[m-ou-se]

That just seems like a bug to me. A (thread local) static &'static str (not mut, not a cell) shouldn't be able to have different values at different times (on the same thread).

[joboet]

Maybe 😄? It's sound however, as there can be no references to the data from the first initialization when the new value is written.

[m-ou-se]

It's sound

Sure, but so would OnceCell be. ^^ And I think users should be able to safely assume that an immutable static never changes. (And as a bonus we get to delete code.)

[m-ou-se]

I did some software archeology and found the original issue and PR that introduced that mem::replace call: #30228 and #30267.

The issue was originally about thread locals, but then most of the discussion was about drop on assignment in general. The issue description suggests:

It should either drop one of the two values (perhaps after putting the TLS slot in "destructor being called" mode) or panic, if a multiple initialization condition is detected.

Without a preference for any of these solutions. The PR also doesn't include a reason for picking one solution over the other.

So I don't think the mem::replace behaviour was purposely picked over another solution (like dropping the other value or panicking).

[workingjubilee]

[m-ou-se]

In this case I don't think anyone actually relies on LazyKeyInner being a OnceOrMaybeTwiceIfYouTryReallyHardCell instead of just a OnceCell. ^^