Thread native name setting #21678

vojtechkral · Jan 27, 2015

I really am not sure I'm doing this right, so here goes nothing...

Also testing this isn't easy. I don't have any other *nix boxes besides a Linux one.

Test code:

use std::thread;
use std::io::timer::sleep;
use std::time::duration::Duration;

fn make_thread<'a>(i: i64) -> thread::JoinGuard<'a, ()>
{
    thread::Builder::new().name(format!("MyThread{}", i).to_string()).scoped(move ||
    {
        println!("Start: {}", i);
        sleep(Duration::seconds(i));
        println!("End: {}", i);
    })
}

fn main()
{
    let mut guards = vec![make_thread(3)];

    for i in 4i64..16
    {
        guards.push(make_thread(i));
    }
}

GDB output on my machine:

(gdb) info threads
  Id   Target Id         Frame 
  15   Thread 0x7fdfbb35f700 (LWP 23575) "MyThread3" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  14   Thread 0x7fdfba7ff700 (LWP 23576) "MyThread4" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  13   Thread 0x7fdfba5fe700 (LWP 23577) "MyThread5" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  12   Thread 0x7fdfba3fd700 (LWP 23578) "MyThread6" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  11   Thread 0x7fdfb8dfe700 (LWP 23580) "MyThread4" 0x00007fdfbb746193 in select () from /usr/lib/libc.so.6
  10   Thread 0x7fdfb8fff700 (LWP 23579) "MyThread7" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  9    Thread 0x7fdfb8bfd700 (LWP 23581) "MyThread8" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  8    Thread 0x7fdfb3fff700 (LWP 23582) "MyThread9" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  7    Thread 0x7fdfb3dfe700 (LWP 23583) "MyThread10" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  6    Thread 0x7fdfb3bfd700 (LWP 23584) "MyThread11" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  5    Thread 0x7fdfb2bff700 (LWP 23585) "MyThread12" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  4    Thread 0x7fdfb29fe700 (LWP 23586) "MyThread13" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  3    Thread 0x7fdfb27fd700 (LWP 23587) "MyThread14" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
  2    Thread 0x7fdfb1bff700 (LWP 23588) "MyThread15" 0x00007fdfbbe35a8d in nanosleep () from /usr/lib/libpthread.so.0
* 1    Thread 0x7fdfbc411800 (LWP 23574) "threads" 0x00007fdfbbe2e505 in pthread_join () from /usr/lib/libpthread.so.0

(I'm not sure why one of the threads is duplicated, but it does that without my patch too...)

rust-highfive · Jan 27, 2015

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pcwalton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

huonw · Jan 27, 2015

huonw reviewed Jan 27, 2015

View changes

huonw · Jan 27, 2015

src/libstd/sys/unix/thread.rs

+    pthread_set_name_np(pthread_self(), cname.as_ptr());
+}
+
+#[cfg(target_os = "macos")]


I think iOS is (somewhat) supported too; does it use pthread_setname_np too?

@huonw It would make sense since the two OSes share the same posix base, but I can't find a direct confirmation... There is a target_os for iOS?

I think it's just "ios".

Maybe @vhbit could confirm/provide advice about the right function here. (IIRC, @vhbit has done most of the iOS work.)

Found it: over here it says pthread_setname_np() is available since OS X 10.6 and iOS 3.2

@vojtechkral @huonw yep, iOS is similar to OS X here, please change it to #[cfg(any(target_os = "macos", target_os = "ios"))]

huonw · Jan 27, 2015

r? @alexcrichton

alexcrichton · Jan 27, 2015

alexcrichton reviewed Jan 27, 2015

View changes

alexcrichton · Jan 27, 2015

src/libstd/sys/unix/thread.rs

+    // because pthread_setname_np() wasn't added until glibc 2.12
+    // PR_SET_NAME since Linux 2.6.9
+    let cname = CString::from_slice(name.as_bytes());
+    prctl(15i32 /* = PR_SET_NAME */, cname.as_ptr() as u64, 0u64, 0u64, 0u64);


Can this use the linkage = "extern_weak" trick instead?

@alexcrichton Sure, which function though? prctl() or pthread_setname_np()?

For pthread_setname_np, but largely only because it looks like it's only defined on newer glibc implementations. This is a bit of a nicety, however, so we probably won't lose too much if it's not available.

alexcrichton · Jan 27, 2015

alexcrichton reviewed Jan 27, 2015

View changes

alexcrichton · Jan 27, 2015

src/libstd/thread.rs

+            match their_thread.name() {
+                Some(thename) => unsafe { imp::set_name(thename.as_slice()); },
+                None => {}
+            }


Could this be folded into thread_info::set to benefit the main thread as well?

vojtechkral · Jan 28, 2015

@alexcrichton Updated as requested. I'll try to find someone to test this on BSD / OS X ...

vhbit · Jan 28, 2015

It should be changed to have target_os = "ios" too, sorry I've missed that before

vojtechkral · Jan 28, 2015

@vhbit I just noticed that too :) Note that the "ios" target should probably accompany most of the other target_os = "macos" occurences as well, but I suppose that's beyond the scope of this PR.

vhbit · Jan 28, 2015

Have no BSD but will test it on OS X

vhbit · Jan 28, 2015

@vojtechkral usually it is although I'm not fast enough to track all the changes :-)

vhbit · Jan 28, 2015

Works fine on OS X

vojtechkral · Jan 28, 2015

@vhbit thanks!

Gankro · Jan 28, 2015

@bors try 7e67

bors · Jan 28, 2015

⌛️ Testing commit 7e67eba with merge 90ed244...

bors · Jan 28, 2015

💔 Test failed - auto-linux-64-x-android-t

Gankro · Jan 28, 2015

Seems to break 64-bit android

vojtechkral · Jan 28, 2015

There was a missing "android" target on the extern, which is weird... I don't understand how it compiled on Android before... edit: It's because the other android cfg test are basically redundant since guard is not enabled for android. It seems the "android" and "ios" target_os tests are kind of a mess :)

Gankro · Jan 28, 2015

@bors try 9ee9

bors · Jan 28, 2015

⌛️ Testing commit 9ee972c with merge f01bc73...

bors · Jan 28, 2015

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

alexcrichton · Jan 28, 2015

@bors: r+ 9ee972c rollup

Thanks!

bors · Jan 28, 2015

⌛️ Testing commit 9ee972c with merge badca40...

bors · Jan 28, 2015

💔 Test failed - auto-linux-64-x-android-t

alexcrichton · Jan 28, 2015

@bors: retry

Thread native name setting, fix #10302

Loading status checks…

ec4981e

rust-highfive assigned pcwalton Jan 27, 2015

rust-highfive assigned alexcrichton and unassigned pcwalton Jan 27, 2015

Thread native name setting, fix #10302

Loading status checks…

c155208

Thread native name setting, fix #10302

Loading status checks…

33a3d6d

Thread native name setting, fix #10302

Loading status checks…

7e67eba

Thread native name setting, fix #10302

Loading status checks…

9ee972c

alexcrichton merged commit 9ee972c into rust-lang:master Jan 31, 2015
1 of 2 checks passed

1 of 2 checks passed

homu Test failed
Details

continuous-integration/travis-ci The Travis CI build passed
Details

vojtechkral deleted the vojtechkral:threads-native-names branch Jan 31, 2015

semarie referenced this pull request Feb 1, 2015
Merged
Adding OpenBSD support to rust #21754

semarie added a commit to semarie/rust that referenced this pull request Feb 1, 2015

openbsd: rebase to master
- incoporate changes introduced by rust-lang#21678

f6414b0

alexcrichton referenced this pull request Feb 3, 2015
Closed
All rust process in `top` are called `<main>` #21911

rust-lang/rust

Join GitHub today

Thread native name setting #21678

Conversation

vojtechkral commented Jan 27, 2015

rust-highfive assigned pcwalton Jan 27, 2015

This comment has been minimized.

rust-highfive commented Jan 27, 2015

huonw reviewed Jan 27, 2015 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

huonw commented Jan 27, 2015

rust-highfive assigned alexcrichton and unassigned pcwalton Jan 27, 2015

alexcrichton reviewed Jan 27, 2015 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

alexcrichton reviewed Jan 27, 2015 View changes

This comment has been minimized.

This comment has been minimized.

vojtechkral commented Jan 28, 2015

This comment has been minimized.

vhbit commented on src/libstd/sys/unix/thread.rs in 33a3d6d Jan 28, 2015

This comment has been minimized.

vojtechkral commented Jan 28, 2015

This comment has been minimized.

vhbit commented Jan 28, 2015

This comment has been minimized.

vhbit commented Jan 28, 2015

This comment has been minimized.

vhbit commented Jan 28, 2015

This comment has been minimized.

vojtechkral commented Jan 28, 2015

This comment has been minimized.

Gankro commented Jan 28, 2015

bors added a commit that referenced this pull request Jan 28, 2015

This comment has been minimized.

bors commented Jan 28, 2015

This comment has been minimized.

bors commented Jan 28, 2015

This comment has been minimized.

Gankro commented Jan 28, 2015

This comment has been minimized.

vojtechkral commented Jan 28, 2015

This comment has been minimized.

Gankro commented Jan 28, 2015

This comment has been minimized.

bors commented Jan 28, 2015

bors added a commit that referenced this pull request Jan 28, 2015

This comment has been minimized.

bors commented Jan 28, 2015

This comment has been minimized.

alexcrichton commented Jan 28, 2015

This comment has been minimized.

bors commented Jan 28, 2015

bors added a commit that referenced this pull request Jan 28, 2015

This comment has been minimized.

bors commented Jan 28, 2015

This comment has been minimized.

alexcrichton commented Jan 28, 2015

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jan 30, 2015

Manishearth added a commit to Manishearth/rust that referenced this pull request Jan 31, 2015

Hide details View details alexcrichton merged commit 9ee972c into rust-lang:master Jan 31, 2015 1 of 2 checks passed

1 of 2 checks passed

vojtechkral deleted the vojtechkral:threads-native-names branch Jan 31, 2015

semarie referenced this pull request Feb 1, 2015

semarie added a commit to semarie/rust that referenced this pull request Feb 1, 2015

alexcrichton referenced this pull request Feb 3, 2015

huonw reviewed Jan 27, 2015

View changes

alexcrichton reviewed Jan 27, 2015

View changes

alexcrichton reviewed Jan 27, 2015

View changes

vhbit commented on `src/libstd/sys/unix/thread.rs` in `33a3d6d` Jan 28, 2015

alexcrichton merged commit `9ee972c` into rust-lang:master Jan 31, 2015
1 of 2 checks passed