Skip to content

Tracking issue for cleaning up std's thread_local implementation details #110897

Open
@m-ou-se

Description

@m-ou-se
Member

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:

tlmap

AAAaaaaAaa

Time to clean it up.

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


Related issues to solve, maybe:

Activity

added
C-cleanupCategory: PRs that clean code up or issues documenting cleanup.
A-thread-localsArea: Thread local storage (TLS)
T-libsRelevant to the library team, which will review and decide on the PR/issue.
on Apr 27, 2023
self-assigned this
on Apr 27, 2023
joboet

joboet commented on Apr 27, 2023

@joboet
Member

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

m-ou-se commented on Apr 27, 2023

@m-ou-se
MemberAuthor

@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

joboet commented on Apr 27, 2023

@joboet
Member

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

m-ou-se

m-ou-se commented on Apr 27, 2023

@m-ou-se
MemberAuthor

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

joboet commented on Apr 27, 2023

@joboet
Member

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

m-ou-se commented on Apr 27, 2023

@m-ou-se
MemberAuthor

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

m-ou-se commented on Apr 27, 2023

@m-ou-se
MemberAuthor

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

workingjubilee commented on Apr 27, 2023

@workingjubilee
Member

"We've all come to rely on undocumented endpoints in your API." "And this gives you power over me?"

added a commit that references this issue on Apr 28, 2023

Rollup merge of rust-lang#110898 - m-ou-se:remove-unused-thread-local…

75be558
m-ou-se

m-ou-se commented on Apr 28, 2023

@m-ou-se
MemberAuthor

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

41 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-thread-localsArea: Thread local storage (TLS)C-cleanupCategory: PRs that clean code up or issues documenting cleanup.T-libsRelevant to the library team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @m-ou-se@thomcc@robertbastian@joboet@workingjubilee

      Issue actions

        Tracking issue for cleaning up std's thread_local implementation details · Issue #110897 · rust-lang/rust