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.

Sign up for GitHub

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 ThreadId for comparing threads #36341

Merged
merged 5 commits into from
Oct 10, 2016
Merged

Add ThreadId for comparing threads #36341

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5bd834b
Add ThreadId for comparing threads
sagebind Sep 8, 2016
6e10e29
Fix tests
sagebind Sep 8, 2016
894ef96
Generate ID using u64 + atomic spinlock
sagebind Oct 5, 2016
e80fd25
Use mutex to guard thread ID counter
sagebind Oct 5, 2016
032bffa
Unlock guard before overflow panic
sagebind Oct 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/libstd/thread/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ use panic;
use panicking;
use str;
use sync::{Mutex, Condvar, Arc};
use sync::atomic::{AtomicUsize, Ordering, ATOMIC_USIZE_INIT};
use sys::thread as imp;
use sys_common::thread_info;
use sys_common::util;
Expand Down Expand Up @@ -524,6 +525,35 @@ pub fn park_timeout(dur: Duration) {
*guard = false;
}

////////////////////////////////////////////////////////////////////////////////
// ThreadId
////////////////////////////////////////////////////////////////////////////////

/// A unique identifier for a running thread.
///
/// A `ThreadId` is an opaque object that has a unique value for each thread
/// that creates one. `ThreadId`s do not correspond to a thread's system-
/// designated identifier.
#[unstable(feature = "thread_id", issue = "21507")]
#[derive(Eq, PartialEq, Copy, Clone)]
pub struct ThreadId(usize);

impl ThreadId {
/// Returns an identifier unique to the current calling thread.
#[unstable(feature = "thread_id", issue = "21507")]
pub fn current() -> ThreadId {
static THREAD_ID_COUNT: AtomicUsize = ATOMIC_USIZE_INIT;
#[thread_local] static mut THREAD_ID: ThreadId = ThreadId(0);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thread_local!?

isn't #[thread_local] unsupported on some platforms?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding was that the thread_local! macro was implemented using #[thread_local], but that may not be the case.

Since the value is atomic already, the complicated mutexes, keys, and locking provided by thread_local! I felt were unnecessary overhead.


unsafe {
if THREAD_ID.0 == 0 {
THREAD_ID.0 = 1 + THREAD_ID_COUNT.fetch_add(1, Ordering::SeqCst);
}
THREAD_ID
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation allows for two distinct threads to have a same ID in some cases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What case is that?

Copy link
Member

@nagisa nagisa Sep 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least

let main_thread_id = ThreadId::current();
loop {
    let thrad2 = thread::spawn(|| { if ThreadId::current() == main_thread_id { println!("it happened") } });
}

To explain it, consider a usize = u16 system. Main thread gets ThreadId(1), then the threads in the loop receive IDs ThreadId(2)...ThreadId(!0). The next thread will wrap the counter assigning ThreadId(1) to the thread, making the still-running main thread and new thread to have colliding IDs. For larger usize systems, the issue will only take longer to manifest itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the only possibility for collision that I thought of. Would it be better to use a u64? If the number of threads you are creating is overflowing a 64 bits (highly unlikely, IMO), then there's not much we can really do, is there? What possible representation could we even use for thread IDs in that case? I can't imagine an OS being able to even handle up to u64-1 threads.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not that Java is something I look up to, but here's how Java implements unique thread IDs:

private static synchronized long nextThreadID() {
    return ++threadSeqNumber;
}

My gut intuition (without doing any calculating) is that It would take thousands of years of running for a loop like that spawning threads to overflow 64 bits. Highly unlikely.

Copy link
Member

@nagisa nagisa Sep 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the number of threads you are creating is overflowing a 64 bits (highly unlikely, IMO), then there's not much we can really do, is there? … I can't imagine an OS being able to even handle up to u64-1 threads.

You’re assuming that all the threads should be alive at the same time in order for the collision to happen, which is simply not true.

Would it be better to use a u64?

Yes it would. At least then it wouldn’t take a usize = u32 target less than a week of spawning threads continuously to arrive at colliding the thread IDs.

What possible representation could we even use for thread IDs in that case?

Using the address of something unique to a thread in the address space common to all threads would be one way to get guarantee that thread IDs would not collide. (see #29448 (comment) though)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A similar issue exists for reference counted pointers. As pointed out in that discussion, you don't actually need to run a googol increment instructions to overflow, if LLVM optimizes the relevant loop (this is certainly possible for Rc and Arc, I'm not so sure about thread creation, that probably is an observable effect in practice). However, memory safety isn't on the line here, so it's probably okay to slap a footnote on the docs and ignore it. (Using u64 might be better for robustness but you sadly can't use atomics for that.)

}
}
}

////////////////////////////////////////////////////////////////////////////////
// Thread
////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -977,6 +1007,16 @@ mod tests {
thread::sleep(Duration::from_millis(2));
}

#[test]
fn test_thread_id_equal() {
assert!(thread::ThreadId::current() == thread::ThreadId::current());
}

#[test]
fn test_thread_id_not_equal() {
assert!(thread::ThreadId::current() != thread::spawn(|| thread::ThreadId::current()).join().unwrap());
}

// NOTE: the corresponding test for stderr is in run-pass/thread-stderr, due
// to the test harness apparently interfering with stderr configuration.
}