Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Safely enforce thread name requirements #127918

Merged
merged 4 commits into from
Jul 20, 2024
Merged
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
39 changes: 28 additions & 11 deletions library/std/src/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ mod tests;
use crate::any::Any;
use crate::cell::{OnceCell, UnsafeCell};
use crate::env;
use crate::ffi::{CStr, CString};
use crate::ffi::CStr;
use crate::fmt;
use crate::io;
use crate::marker::PhantomData;
Expand Down Expand Up @@ -487,11 +487,7 @@ impl Builder {
amt
});

let my_thread = name.map_or_else(Thread::new_unnamed, |name| unsafe {
Thread::new(
CString::new(name).expect("thread name may not contain interior null bytes"),
)
});
let my_thread = name.map_or_else(Thread::new_unnamed, |name| Thread::new(name.into()));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let my_thread = name.map_or_else(Thread::new_unnamed, |name| Thread::new(name.into()));
let my_thread = name.map_or_else(Thread::new_unnamed, Thread::new);

let their_thread = my_thread.clone();

let my_packet: Arc<Packet<'scope, T>> = Arc::new(Packet {
Expand Down Expand Up @@ -1273,10 +1269,34 @@ impl ThreadId {
/// The internal representation of a `Thread`'s name.
enum ThreadName {
Main,
Other(CString),
Other(ThreadNameString),
Unnamed,
}

// This module ensures private fields are kept private, which is necessary to enforce the safety requirements.
mod thread_name_string {
use crate::ffi::{CStr, CString};

/// Like a `String` it's guaranteed UTF-8 and like a `CString` it's null terminated.
pub(crate) struct ThreadNameString {
inner: CString,
}
impl core::ops::Deref for ThreadNameString {
type Target = CStr;
fn deref(&self) -> &CStr {
&self.inner
}
}
impl From<String> for ThreadNameString {
fn from(s: String) -> Self {
Self {
inner: CString::new(s).expect("thread name may not contain interior null bytes"),
}
}
}
}
pub(crate) use thread_name_string::ThreadNameString;

/// The internal representation of a `Thread` handle
struct Inner {
name: ThreadName, // Guaranteed to be UTF-8
Expand Down Expand Up @@ -1316,10 +1336,7 @@ pub struct Thread {

impl Thread {
/// Used only internally to construct a thread object without spawning.
///
/// # Safety
/// `name` must be valid UTF-8.
pub(crate) unsafe fn new(name: CString) -> Thread {
pub(crate) fn new(name: ThreadNameString) -> Thread {
unsafe { Self::new_inner(ThreadName::Other(name)) }
}

Expand Down
Loading