Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std::thread::LocalKeyState: Add fourth Initializing state #43491

Closed
joshlf opened this issue Jul 26, 2017 · 1 comment
Closed

std::thread::LocalKeyState: Add fourth Initializing state #43491

joshlf opened this issue Jul 26, 2017 · 1 comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@joshlf
Copy link
Contributor

joshlf commented Jul 26, 2017

Right now, LocalKeyState contains three states: Uninitialized, Valid, and Destroyed.

Unfortunately, this is not enough to cover all cases. In particular, consider the following code (adapted from some code I wrote as part of writing a global allocator - to motivate that this is a real-world example):

fn local_or_global(foo: Foo) -> Bar {
    match LOCAL.state() {
        LocalKeyState::Uninitialized | LocalKeyState::Valid => LOCAL.with(|local| do_computation(foo, local)),
        LocalKeyState::Destroyed => do_computation(foo, &GLOBAL),
    }
}

The idea here is to ensure that the local key hasn't yet been destroyed - and if it has, to fall back on some global state instead. If the key is valid, then obviously this is fine. If the key is uninitialized, then with should perform initialization, which should also be fine...

unless local_or_global is called from the initialization routine. This is the case for the allocator code I'm working on since initializing TLS requires performing allocations. In that case, you end up with infinite recursion and eventually a stack overflow.

My proposal is the following: add a fourth Initializing state. When with is first called and the key needs to be initialized, it is first moved into the Initializing state. When initialization completes, it is subsequently moved into the Valid state. This allows code that accesses the key to detect whether it's being called from inside the initializer, and behave appropriately in that case (in particular, by not accessing the key and thus recursing the initialization).

The concrete issue I (along with my collaborator on this project, @ezrosent) have run into is that initializing TLS depends on allocation, which depends on TLS, which depends on... In general, this is a problem whenever a TLS key's initialization routine transitively depends on itself (either directly, or through an arbitrarily long cycle of other TLS key dependencies - for example, we've seen similar issues with Destroyed keys when using Crossbeam's epoch-based memory reclamation, which also uses TLS under the hood).

Alternatives

It might be tempting to think that just making sure you're in the Valid state is a sufficient (if sub-optimal) solution to this problem (and in fact, I thought the same thing for a while). Unfortunately, this solves a chicken-and-egg problem by just deciding that chickens can't exist. By only being willing to access a TLS key in the Valid state, you preclude ever calling code that will perform initialization, and thus you'll never move to the Valid state in the first place.

@Mark-Simulacrum Mark-Simulacrum added C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 26, 2017
@joshlf
Copy link
Contributor Author

joshlf commented Sep 7, 2017

Per the discussion in #43550, we've decided not to make this change. Closing in favor of #44396.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants