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

Add a numeric ID to threads #29448

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
Pass ids from sys-specific threads
  • Loading branch information
remram44 committed Oct 29, 2015
commit 6149a2ad00f030ec083cc9e527a12385a5531b44
4 changes: 2 additions & 2 deletions src/libstd/sys/common/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ use alloc::boxed::FnBox;
use libc;
use sys::stack_overflow;

pub unsafe fn start_thread(main: *mut libc::c_void) {
pub unsafe fn start_thread(main: *mut libc::c_void, id: u32) {
// Next, set up our stack overflow handler which may get triggered if we run
// out of stack.
let _handler = stack_overflow::Handler::new();

// Finally, let's run some code.
Box::from_raw(main as *mut Box<FnBox()>)()
Box::from_raw(main as *mut Box<FnBox(u32)>)(id)
Copy link
Member

Choose a reason for hiding this comment

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

I do not understand why’d you want to pass the thread id as an argument into the thread instead of e.g. providing a static method on Thread which calls pthread_self.

}
4 changes: 2 additions & 2 deletions src/libstd/sys/unix/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ unsafe impl Send for Thread {}
unsafe impl Sync for Thread {}

impl Thread {
pub unsafe fn new<'a>(stack: usize, p: Box<FnBox() + 'a>)
pub unsafe fn new<'a>(stack: usize, p: Box<FnBox(u32) + 'a>)
-> io::Result<Thread> {
let p = box p;
let mut native: libc::pthread_t = mem::zeroed();
Expand Down Expand Up @@ -71,7 +71,7 @@ impl Thread {
};

extern fn thread_start(main: *mut libc::c_void) -> *mut libc::c_void {
unsafe { start_thread(main); }
unsafe { start_thread(main, pthread_self() as u32); }
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong.

POSIX.1 allows an implementation wide freedom in choosing the type used to represent a thread ID; for example, representation using either an arithmetic type or a structure is permitted.

Copy link
Member

Choose a reason for hiding this comment

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

So should we use a typedef around what a thread ID is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Linux pthread_t is unsigned long, thus u64 on amd64. There is no portable way to get a numeric ID from pthreads (pthread_getthreadid_np is not portable).

I'm starting to think that using the address of the Inner struct is way simpler and would totally work (it's what I used for #29447) if we want "some kind of ID" and don't care for the OS-provided ID.

Copy link
Member

Choose a reason for hiding this comment

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

On Windows what I want is some sort of OS provided ID. Either the thread HANDLE or the thread ID which is a DWORD. Anything else would be useless to me as I wouldn't be able to use it in OS functions to manipulate that thread like queuing APCs to it.

Copy link
Member

Choose a reason for hiding this comment

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

Nevermind, I'm fine with this just providing some sort of unique numerical ID that has no meaning other than to distinguish threads. I'll work on a PR to add OS specific APIs for getting the HANDLE of a thread.

ptr::null_mut()
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/sys/windows/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Thread {
};

extern "system" fn thread_start(main: *mut libc::c_void) -> DWORD {
unsafe { start_thread(main); }
unsafe { start_thread(main, 42); }
0
}
}
Expand Down
11 changes: 7 additions & 4 deletions src/libstd/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,11 @@ impl Builder {
= Arc::new(UnsafeCell::new(None));
let their_packet = my_packet.clone();

let main = move || {
let main = move |id: u32| {
if let Some(name) = their_thread.name() {
imp::Thread::set_name(name);
}
their_thread.inner.id = id;
unsafe {
thread_info::set(imp::guard::current(), their_thread);
let mut output = None;
Expand All @@ -276,10 +277,12 @@ impl Builder {
}
};

let native = unsafe {
try!(imp::Thread::new(stack_size, Box::new(main)))
};
my_thread.inner.id = native.id();
Ok(JoinHandle(JoinInner {
native: unsafe {
Some(try!(imp::Thread::new(stack_size, Box::new(main))))
},
native: Some(native),
thread: my_thread,
packet: Packet(my_packet),
}))
Expand Down