std::thread: set_name implementation proposal for vxWorks.

[devnexen]

No description provided.

[rustbot]

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[workingjubilee]

If this type is only used for vxworks, I think we should just add it here, honestly.

[devnexen]

well because STATUS is just a c_int, and so on ... I was thinking they might serve later on in other places.

[tgross35]

It looks like libc already has OK which is defined as a c_int https://github.com/rust-lang/libc/blob/0e28c864c25d2e9b0ab082947445efccef213da4/src/vxworks/mod.rs#L616. So I think debug_assert_eq!(res, libc::OK); should already work? We just discard the error condition so that isn't needed.

If a status type is used in other places, we should probably update libc first and then do the changes at once. Maybe just mark this // FIXME(libc): ... to make it more searchable.

[devnexen]

Cool I missed the OK. I forgot this PR, while at it, I ll add STATUS type too.

[tgross35]

This looks fine to me with something to address Jubilee's comment.

@biabbas does this look correct?

@devnexen not sure if you are working specifically on VxWorks or just RTOSs in general, but you may like to add yourself as a VxWorks maintainer in https://github.com/biabbas/rustLangRust/blob/bf9a4d6d43ef03b0e300d48772dbfe3d442f5f98/src/doc/rustc/src/platform-support/vxworks.md if you have any specific familiarity there.

[tgross35]

One last thing, this note is no longer accurate

rust/library/std/src/sys/pal/unix/thread.rs

Line 251 in 308c8e8

// Newlib, Emscripten, and VxWorks have no way to set a thread name.

[tgross35]

        // FIXME(vxworks): if the minimum supported VxWorks version is >= 7 is chosen, the maximum length can change to 31.

Is something like this accurate?

[devnexen]

sure

[tgross35]

Thanks for the changes, this seems fine to me until rust-lang/libc#3780 merges.

@bors r+

[bors]

📌 Commit b9e8e99 has been approved by tgross35

It is now in the queue for this repository.

[biabbas]

@devnexen Did you test these changes?

[devnexen]

did you encounter any issue ?

[biabbas]

did you encounter any issue ?
Yes.

[tgross35]

Can you be more descriptive please?

[biabbas]

By my understanding, the name on thread builder calls set_name.
So I'm using this code to test:

use std::thread;
use std::time::Duration;

fn main() {
    // Create and start the thread with a name
    let handle = thread::Builder::new()
        .name("MyTestThread_1".to_string())
        .spawn(|| {
            // Print the current thread's name
            // Note: The thread's name is not directly accessible in Rust.
            // However, you can use a workaround with thread-local storage or other means.
            // Here, we're simulating the concept of using a named thread.
            let binding = thread::current();
            let name = binding.name().unwrap_or("Unnamed");
            println!("Thread Name in threadFunction: {}", name);
            thread::sleep(Duration::from_secs(1)); // Sleep to simulate work
        })
        .expect("Failed to create thread");
    // Wait for the thread to finish
    handle.join().expect("Thread panicked");
    // Verify the name in the main thread
    let binding = thread::current();
    let main_thread_name = binding.name().unwrap_or("Unnamed");
    println!("Main Thread Name: {}", main_thread_name);
}

Output with rust 1.80:
[vxWorks *]# ./set_name.vxe
Thread Name in threadFunction: MyTestThread_1
Main Thread Name: main

This already works in rust 1.80, where these changes are not made.
I might be wrong about this. Should the function be tested some other way?

[devnexen]

You may be able to try implementing a thread get_name locally, the vxWorks function is simply called taskName(libc::TASK_ID) -> *mut libc::c_char; (by head). Just a suggestion :)

[biabbas]

You may be able to try implementing a thread get_name locally, the vxWorks function is simply called taskName(libc::TASK_ID) -> *mut libc::c_char; (by head). Just a suggestion :)

But I'm sure the example I gave must be calling set_name. I'm confused on how the threads name are set without a set_name implementation.

[devnexen]

I may be wrong but aren't you simply accessing Thread::name() member in your code ? so it does not come from the thread then. Again might be misreading.

[biabbas]

.name("MyTestThread_1".to_string())

this line is supposed to call set_name internally.

[biabbas]

Can you be more descriptive please?

1. Build errors
   
   After resolving this,
   

2. After resolving the build error, while testing the function I added a print statement in the end of the function set name.
   This was the output with this configuration : 49c3476

[devnexen]

.name("MyTestThread_1".to_string())

this line is supposed to call set_name internally.

I meant this let name = binding.name().unwrap_or("Unnamed");

[biabbas]

.name("MyTestThread_1".to_string())

this line is supposed to call set_name internally.

I meant this let name = binding.name().unwrap_or("Unnamed");

Output with rust 180 with print debug statement added in the empty set name implementation:

[devnexen]

what happens if you comment ?

let status = unsafe { f(libc::taskIdSelf(), name.as_mut_ptr()) };
            debug_assert_eq!(status, libc::OK);

[biabbas]

let status = unsafe { f(libc::taskIdSelf(), name.as_mut_ptr()) };

That is how I resolved the build errors.
Refer these:

1f1dbb5
0e5b18a
49c3476

All the runtime output shared were captured after a successful build.

[biabbas]

I would like to revert these changes, cause I see the thread name being set without a set_name implementation. Also the set_name implementation causes runtime failures. And note the current code has errors that cause the build to fail.

[biabbas]

@devnexen I'm grateful that you put in effort, but the set name implementation seems unnecessary after this exercise .

[devnexen]

So you want to revert this set_name but you re ok keeping available_parallelism (but still need fixing its one line error build) ? If that s the case, I m ok too to revert.

[biabbas]

But that still leaves the question of how the thread name is set to the name specified in builder class with an empty set_name implementation.

[devnexen]

did you try with a long thread name ? If the thread name is set it should be truncated.

[biabbas]

did you try with a long thread name ? If the thread name is set it should be truncated.
Yes. It's not truncated.
I removed println from set_name and the runtime issue was solved. Do you know why?

set_name_1801.vxe was compiled with rustc 1.80, which did not have an implementation for setname
set_name_1802.vxe was compiled with rustc 1.82 which had setname implementation.

I'll just fix the build failures on both set name and available_parallelism and raise a pr.