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

Tracking issue for thread_local stabilization #29594

Open
3 of 6 tasks
aturon opened this issue Nov 5, 2015 · 74 comments
Open
3 of 6 tasks

Tracking issue for thread_local stabilization #29594

aturon opened this issue Nov 5, 2015 · 74 comments
Labels
A-ffi Area: Foreign Function Interface (FFI) A-thread-locals Area: Thread local storage (TLS) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-thread_local `#![feature(thread_local)]` S-tracking-impl-incomplete Status: The implementation is incomplete. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@aturon
Copy link
Member

aturon commented Nov 5, 2015

The #[thread_local] attribute is currently feature-gated. This issue tracks its stabilization.

Known problems:

@aturon aturon added T-lang Relevant to the language team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels Nov 5, 2015
@aturon

This comment has been minimized.

@alexcrichton

This comment has been minimized.

@alexcrichton
Copy link
Member

A note, we've since implemented cfg(target_thread_local) which is in turn itself feature gated, but this may ease the "this isn't implemented on all platforms" worry.

@alexandermerritt
Copy link
Contributor

Hi! Is there any update on the status? Nightly still requires statics to be Sync. I tried with:

rustc 1.13.0-nightly (acd3f796d 2016-08-28)
binary: rustc
commit-hash: acd3f796d26e9295db1eba1ef16e0d4cc3b96dd5
commit-date: 2016-08-28
host: x86_64-unknown-linux-gnu
release: 1.13.0-nightly

@mneumann
Copy link
Contributor

@alexcrichton Any news on #[thread_local] becoming stabilized? AFAIK, at the moment it is impossible on DragonFly to access errno variable from stable code, other than directly from libstd. This blocks crates like nix on DragonFly, which want to access errno as well, but libstd is not exposing it, and stable code is not allowed to use feature(thread_local).

@alexcrichton
Copy link
Member

@mneumann no, no progress. I'd recommend a C shim for now.

@mneumann
Copy link
Contributor

@alexcrichton thanks. I am doing a shim now https://github.com/mneumann/errno-dragonfly-rs.

@Boiethios
Copy link

Boiethios commented Sep 8, 2017

The optimizations are too aggressive ;)

See this code:

#![feature(thread_local)]

#[thread_local]
pub static FOO: [&str; 1] = [ "Hello" ];

fn change_foo(s: &'static str) {
    FOO[0] = s;
}

fn main() {
    println!("{}", FOO[0]);
    change_foo("Test");
    println!("{}", FOO[0]);
}

The compiler does not detect the side effect in change_foo and removes the call in release. The output is:

Hello
Hello

@alexcrichton
Copy link
Member

cc @eddyb, @Boiethios your example shouldn't actually compile because it should require static mut, not just static

@Boiethios
Copy link

It compiles with the last nightly rustc.

@eddyb
Copy link
Member

eddyb commented Sep 8, 2017

Oh, drat, this is from my shortening of the lifetime, i.e

// `#[thread_local]` statics may not outlive the current function.
for attr in &self.tcx.get_attrs(def_id)[..] {
if attr.check_name("thread_local") {
return Ok(self.cat_rvalue_node(id, span, expr_ty));
}
}

@nikomatsakis what should we do here? I want a static lvalue, with a non-'static lifetime.

@cramertj
Copy link
Member

#18001, #17954 and #18712 were fixed by #43746.

@alexcrichton @eddyb Do you know any other blockers, or is this ready for stabilization?

@eddyb
Copy link
Member

eddyb commented Jan 17, 2018

There's some emulation clang does IIRC, that we might want to do ourselves, to support #[thread_local] everywhere.

And there's #47053 which results from my initial attempt to limit references to thread-local statics to the function they were created in.

@alexcrichton
Copy link
Member

@cramertj I've personally been under the impression that we're holding out on stabilizing this for as long as possible. We've stabilized very few (AFAIK) platform-specific attributes like this and I at least personally haven't ever looked to hard into stabilizing this.

One blocker (in my mind at least) is what @eddyb mentioned where this is a "portable" attribute yet LLVM has a bunch of emulation on targets that don't actually support it (I think MinGW is an example). I don't believe we should allow the attribute on such targets, but we'd have to do a lot of investigation to figure out those targets.

Is there motivation for stabilizing this though? That'd certainly provide some good motivation for digging out any remaining issues and looking at it very closely. I'd just personally been under the impression that there's little motivation to stabilize this other than it'd be a "nice to have" in situations here and there.

@Amanieu
Copy link
Member

Amanieu commented Jan 17, 2018

I am using #[thread_local] in my code in a no_std context: I allocate space for the TLS segment and set up the architecture TLS register to point to it. Therefore I think that it is important to expose the #[thread_local] attribute so that it can at least be used by low-level code.

The only thing that I'm not too happy about is that Sync is no longer required for #[thread_local]: this makes it more difficult to write signal-safe code, since all communication between signal handlers and the main thread should be done through atomic types.

@eddyb
Copy link
Member

eddyb commented Jan 17, 2018

@Amanieu Signal/interrupt-safe code has other requirements which are not satisfied by current Rust.

@Amanieu
Copy link
Member

Amanieu commented Jan 17, 2018

@eddyb Not really, all you need to do is treat the signal handler as a separate thread of execution. The only requirement you need to enforce safety is that objects which are accessed by both the main thread and the signal handler must be Sync.

Of course you can still cause deadlocks if the main thread and signal handler both try to grab the same lock, but that's not a safety issue.

dcuddeback added a commit to dcuddeback/serial-rs that referenced this issue Jun 22, 2018
DragonFlyBSD uses a thread-local variable for errno. libstd uses a
feature (`thread_local`) which is not stablized [1].

[1] rust-lang/rust#29594
@dtolnay
Copy link
Member

dtolnay commented Oct 8, 2018

@cramertj regarding remaining blockers, I would consider the behavior that @Boiethios reported above needs to be fixed before stabilization. I filed #54901 to follow up.

jackpot51 pushed a commit to redox-os/ion that referenced this issue Dec 29, 2018
__dfly_error() was removed from Rust many years ago.

DragonFly uses a thread-local errno variable, but #[thread_local] is
feature-gated and not available in stable Rust as of this writing
(Rust 1.31.0). We have to use a C extension to access it.

Tracking issue for `thread_local` stabilization:

    rust-lang/rust#29594

Once this becomes stable, we can simply use:

    extern { #[thread_local] static errno: c_int; }
@peter-kehl
Copy link
Contributor

#[thread_local] causes an Internal Compiler Error if used with proc macro-specific types (like proc_macro::{Delimiter, Group, Ident, Punct, Spacing, Span, TokenStream, TokenTree}). Could we either

  • document that it's not for proc macro-specific types, or
  • have a tracking issue for this, please?

@cybersoulK
Copy link

#115621

segment fault on windows. is it a misuse of the feature, or a bug?

@RalfJung
Copy link
Member

RalfJung commented Sep 7, 2023

Would it makes sense to just reject the attribute on targets where target_thread_local is not set?

@RalfJung
Copy link
Member

RalfJung commented Sep 7, 2023

#[thread_local] causes an Internal Compiler Error if used with proc macro-specific types (like proc_macro::{Delimiter, Group, Ident, Punct, Spacing, Span, TokenStream, TokenTree}). Could we either

This has nothing to do with this tracking issue; it's about the entire proc-macro system being incompatible with thread-local state (no matter how that state got implemented).

This tracking issue is about thread-local state specifically implemented via the native mechanism of the linker (as opposed to something like pthread keys). The thread_local! macro has different implementations depending on the target; sometimes it uses linker-native thread-locals (internally using the feature tracked here), sometimes it uses slower but less fragile run-time OS-provided mechanisms.

@ChrisDenton
Copy link
Contributor

I wonder if there's a path towards a minimal thread_local stabilization by being very restrictive?

E.g.:

  • all access is by value only
  • only allow Copy types (or maybe even restrict it to only a subset of primitives)
  • don't allow lifetimes
  • don't allow composing thread_local with other lang attributes (they may interact in "interesting" ways)
  • make thread_local error if the platform does not support it. This would imply also stabilizing target_thread_local but maybe the name should be subject to bikeshedding first (e.g. has_static_thread_local) to be clear that it may still have runtime thread locals.

@RalfJung
Copy link
Member

What is the motivation for that? Doesn't thread_local! { static NAME = const { ... } } suffice?

@ChrisDenton
Copy link
Contributor

no_std mainly

@RalfJung
Copy link
Member

RalfJung commented Dec 22, 2023

Does that form of the macro need anything that requires std? Could there be a core::thread_local! { ... } macro that makes that part of the functionality available without std (i.e., it would require const blocks)?

@ChrisDenton
Copy link
Contributor

We could make a macro I guess. It'd be pretty redundant though when/if thread_local proper is stabilized. Unless we decide to just have a macro and not the attribute.

@RalfJung
Copy link
Member

RalfJung commented Dec 22, 2023

The API provided by #[thread_local] static and const-thread_local is pretty different I think. So your proposal does introduce redundancy that we currently don't have. There'd be two stable ways to do the same thing and two completely different mechanisms to ensure the necessary restrictions (such as "no 'static references").

@ChrisDenton
Copy link
Contributor

The "two ways to do things" will occur whatever happens unless we simply don't stabilize #[thread_local] static.

@RalfJung
Copy link
Member

RalfJung commented Dec 22, 2023

That is true. If the usecases are covered I don't see a reason to stabilize #[thread_local] static. It could be transitioned to an internal feature, an implementation detail of the public macros.

@ChrisDenton
Copy link
Contributor

core::thread_local! would also need LocalKey moved into core::thread (or an equivalent to it). One thing that thread_local! forces that #[thread_local] doesn't is going through a shared reference. There would also still need to be a way to know if thread_local is supported in no_std.

@newpavlov
Copy link
Contributor

What is the motivation for that? Doesn't thread_local! { static NAME = const { ... } } suffice?

IIRC generated assembly for thread_local! was quite ugly when compared to equivalent #[thread_local] code. I haven't measured performance impact and do not know if it's possible to work around it with std changes, but it's still an example of non-zero-costness. Also, #[thread_local]-based code simply looks nicer and more ergonomic.

@ChrisDenton
Copy link
Contributor

ChrisDenton commented Dec 22, 2023

For the new thread_local! { static FOO: ... = const { ... } syntax it should all be optimized down to a minimum. If not then we should really try to fix that.

EDIT: assuming the type of FOO has no drop, of course.

@RalfJung
Copy link
Member

Also, #[thread_local]-based code simply looks nicer and more ergonomic.

If they only work for Copy types and you can't take any references (and hence also not call any &self/&mut self methods), I am not sure if that's still true.

And taking references would be unsound.

@bjorn3
Copy link
Member

bjorn3 commented Dec 22, 2023

And taking references would be unsound.

Borrowck limits those references to the current function already I believe.

@ChrisDenton
Copy link
Contributor

Yes. The goal would be to eventually make full #[thread_local] stabilized (once bugs, etc are fixed). But that's not happening any time soon and in the meantime a minimal stabilization would be useful.

@RalfJung
Copy link
Member

Oh interesting, I wasn't aware that borrowck had special treatment for thread_local statics.

@chorman0773
Copy link
Contributor

chorman0773 commented Dec 22, 2023

I'd be very interested in seeing this stablized.
I use #[thread_local] in the interface code for my WIP OS, as an import/export for thread_local statics (https://github.com/LiliumOS/lilium-sys/blob/main/src/sys/io.rs#L44-L54). The handles here are thread-local (as handles themselves are thread-local resources in the OS) that are initialized by the USI (userspace standard interface) when you create a thread (This specific case is limited to a Copy type and you probably won't be be borrowing the handles often at all).

In general, being able to import an external TLS var without shim code written in C is useful (for example, if you want to grab __errno). This cannot be done with LocalKey and std::thread_local!.
#[thread_local] would also enable using TLS in a no-std context, such as a kernel.

@newpavlov
Copy link
Contributor

@RalfJung

If they only work for Copy types and you can't take any references (and hence also not call any &self/&mut self methods), I am not sure if that's still true.

How about introducing a pointer-based variant of #[thread_local]? Something like this:

// Creates TLS value which stores 42 and creates "pointer" `FOO` to it.
#[thread_local_ptr]
static FOO: *mut u64 = 42u64;

fn increment_foo() {
    // SAFETY: reference to `FOO`does not escape execution thread
    let foo: &mut u64  = unsafe { &mut *FOO };
    *foo += 1;
}

@RalfJung
Copy link
Member

@newpavlov it seems someone already taught the borrow checker about thread_local so taking references to them is actually sound. That's pretty cool.

I don't like there being this API duplication and inconsistency between that and the thread_local! macro, but that's up to libs-api to figure out.

We'd have to be rather careful where we enable this feature; in the past, I think on some Windows targets we switched back-and-forth between "true" thread-local statics and some other implementation for the thread_local! macro. The macro gives us the flexibility to do that; #[thread_local] does not, so once we allow it somewhere, if we later figure out there is some platform issue then we are in trouble.

I'm slightly bothered by thread-local statics pretending to be statics. They have very little in common with regular static in terms of their semantics. &FOO isn't even a constant, it is a function. We do reflect this in the MIR at least so bugs due to this are hopefully unlikely. And I don't have a proposal for a better syntax either so 🤷

@Amanieu
Copy link
Member

Amanieu commented Jan 2, 2024

I have a few use cases for #[thread_local] that can't be satisfied by the standard library's thread_local! macro:

  • The code base is a no_std binary which doesn't depend on libc (it has its own TLS/stack initialization code).
  • It exports #[thread_local] variables for use by C code (errno).
  • It used to #[thread_local] variables in inline assembly (by symbol name), although that code has since been refactored and it not longer does that. It is possible to know the exact instruction sequence to use with the symbol because the whole code base is compiled with -Z tls-model=local-exec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ffi Area: Foreign Function Interface (FFI) A-thread-locals Area: Thread local storage (TLS) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. F-thread_local `#![feature(thread_local)]` S-tracking-impl-incomplete Status: The implementation is incomplete. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests