-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add a numeric ID to threads #29448
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
Add a numeric ID to threads #29448
Conversation
r? @brson (rust_highfive has picked a reviewer for you, use r? to override) |
/// | ||
/// This ID is unique across running threads, and a thread that has stopped | ||
/// might see its ID reused. | ||
pub fn id(&self) -> u32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this PR is accepted, this will need to be marked #[unstable]
and have a tracking issue opened. (See catch_panic
in this module for an example.)
I think that there's a few pieces that may want to be tweaked here:
I'm also not sure that we want to use the exact same ID as the underlying OS. As @nagisa points out we can't rely on |
Getting the ID of the current thread is trivial, especially on Windows, so there shouldn't be any reason for |
☔ The latest upstream changes (presumably #29546) made this pull request unmergeable. Please resolve the merge conflicts. |
Haven't looked at this in detail, but because the Rust thread type outlives the system thread, it's possible for comparison operations to produce incorrect results if the system reuses the thread id. |
The libs team discussed this in triage today, and in addition to @brson's comment we also had thinking along the lines of:
|
Closing due to inactivity, but feel free to resubmit with the comments on this and #29447 addressed! |
Add ThreadId for comparing threads This adds the capability to store and compare threads with the current calling thread via a new struct, `std::thread::ThreadId`. Addresses the need outlined in issue #21507. This avoids the need to add any special checks to the existing thread structs and does not rely on the system to provide an identifier for a thread, since it seems that this approach is unreliable and undesirable. Instead, this simply uses a lazily-created, thread-local `usize` whose value is copied from a global atomic counter. The code should be simple enough that it should be as much reliable as the `#[thread_local]` attribute it uses (however much that is). `ThreadId`s can be compared directly for equality and have copy semantics. Also see these other attempts: - #29457 - #29448 - #29447 And this in the RFC repo: rust-lang/rfcs#1435
See #21507
Because we only get an ID once the thread has been spawned, and at that point the Inner struct is referenced from both the thread (which is gonna store it in the threadlocal thread_info) and the parent (which returns it), it was much easier to add it to Thread than to Inner. It is a single u32 so I'm not sure the memory usage is an issue, but it is a little awkward to have id in Thread and name in Inner?
Let me know.