Skip to content

Commit

Permalink
Rollup merge of #105903 - joboet:unify_parking, r=m-ou-se
Browse files Browse the repository at this point in the history
Unify id-based thread parking implementations

Multiple platforms currently use thread-id-based parking implementations (NetBSD and SGX[^1]). Even though the strategy does not differ, these are duplicated for each platform, as the id is encoded into an atomic thread variable in different ways for each platform.

Since `park` is only called by one thread, it is possible to move the thread id into a separate field. By ensuring that the field is only written to once, before any other threads access it, these accesses can be unsynchronized, removing any restrictions on the size and niches of the thread id.

This PR also renames the internal `thread_parker` modules to `thread_parking`, as that name now better reflects their contents. I hope this does not add too much reviewing noise.

r? `@m-ou-se`

`@rustbot` label +T-libs

[^1]: SOLID supports this as well, I will switch it over in a follow-up PR.
  • Loading branch information
compiler-errors authored Dec 31, 2022
2 parents 5570cda + 898302e commit ff3326d
Show file tree
Hide file tree
Showing 19 changed files with 207 additions and 240 deletions.
2 changes: 1 addition & 1 deletion library/std/src/sys/sgx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub mod process;
pub mod stdio;
pub mod thread;
pub mod thread_local_key;
pub mod thread_parker;
pub mod thread_parking;
pub mod time;

mod condvar;
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/sgx/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ mod task_queue {
/// execution. The signal is sent once all TLS destructors have finished at
/// which point no new thread locals should be created.
pub mod wait_notify {
use super::super::thread_parker::Parker;
use crate::pin::Pin;
use crate::sync::Arc;
use crate::sys_common::thread_parking::Parker;

pub struct Notifier(Arc<Parker>);

Expand All @@ -87,14 +87,14 @@ pub mod wait_notify {
/// called, this will return immediately, otherwise the current thread
/// is blocked until notified.
pub fn wait(self) {
// This is not actually `unsafe`, but it uses the `Parker` API,
// which needs `unsafe` on some platforms.
// SAFETY:
// This is only ever called on one thread.
unsafe { Pin::new(&*self.0).park() }
}
}

pub fn new() -> (Notifier, Waiter) {
let inner = Arc::new(Parker::new_internal());
let inner = Arc::new(Parker::new());
(Notifier(inner.clone()), Waiter(inner))
}
}
Expand Down
107 changes: 0 additions & 107 deletions library/std/src/sys/sgx/thread_parker.rs

This file was deleted.

23 changes: 23 additions & 0 deletions library/std/src/sys/sgx/thread_parking.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
use super::abi::usercalls;
use crate::io::ErrorKind;
use crate::time::Duration;
use fortanix_sgx_abi::{EV_UNPARK, WAIT_INDEFINITE};

pub type ThreadId = fortanix_sgx_abi::Tcs;

pub use super::abi::thread::current;

pub fn park(_hint: usize) {
usercalls::wait(EV_UNPARK, WAIT_INDEFINITE).unwrap();
}

pub fn park_timeout(dur: Duration, _hint: usize) {
let timeout = u128::min(dur.as_nanos(), WAIT_INDEFINITE as u128 - 1) as u64;
if let Err(e) = usercalls::wait(EV_UNPARK, timeout) {
assert!(matches!(e.kind(), ErrorKind::TimedOut | ErrorKind::WouldBlock))
}
}

pub fn unpark(tid: ThreadId, _hint: usize) {
let _ = usercalls::send(EV_UNPARK, Some(tid));
}
2 changes: 1 addition & 1 deletion library/std/src/sys/unix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub mod stdio;
pub mod thread;
pub mod thread_local_dtor;
pub mod thread_local_key;
pub mod thread_parker;
pub mod thread_parking;
pub mod time;

#[cfg(target_os = "espidf")]
Expand Down
113 changes: 0 additions & 113 deletions library/std/src/sys/unix/thread_parker/netbsd.rs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ unsafe impl Sync for Parker {}
unsafe impl Send for Parker {}

impl Parker {
pub unsafe fn new(parker: *mut Parker) {
pub unsafe fn new_in_place(parker: *mut Parker) {
let semaphore = dispatch_semaphore_create(0);
assert!(
!semaphore.is_null(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ cfg_if::cfg_if! {
pub use darwin::Parker;
} else if #[cfg(target_os = "netbsd")] {
mod netbsd;
pub use netbsd::Parker;
pub use netbsd::{current, park, park_timeout, unpark, ThreadId};
} else {
mod pthread;
pub use pthread::Parker;
Expand Down
52 changes: 52 additions & 0 deletions library/std/src/sys/unix/thread_parking/netbsd.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
use crate::ffi::{c_int, c_void};
use crate::ptr;
use crate::time::Duration;
use libc::{_lwp_self, clockid_t, lwpid_t, time_t, timespec, CLOCK_MONOTONIC};

extern "C" {
fn ___lwp_park60(
clock_id: clockid_t,
flags: c_int,
ts: *mut timespec,
unpark: lwpid_t,
hint: *const c_void,
unparkhint: *const c_void,
) -> c_int;
fn _lwp_unpark(lwp: lwpid_t, hint: *const c_void) -> c_int;
}

pub type ThreadId = lwpid_t;

#[inline]
pub fn current() -> ThreadId {
unsafe { _lwp_self() }
}

#[inline]
pub fn park(hint: usize) {
unsafe {
___lwp_park60(0, 0, ptr::null_mut(), 0, ptr::invalid(hint), ptr::null());
}
}

pub fn park_timeout(dur: Duration, hint: usize) {
let mut timeout = timespec {
// Saturate so that the operation will definitely time out
// (even if it is after the heat death of the universe).
tv_sec: dur.as_secs().try_into().ok().unwrap_or(time_t::MAX),
tv_nsec: dur.subsec_nanos().into(),
};

// Timeout needs to be mutable since it is modified on NetBSD 9.0 and
// above.
unsafe {
___lwp_park60(CLOCK_MONOTONIC, 0, &mut timeout, 0, ptr::invalid(hint), ptr::null());
}
}

#[inline]
pub fn unpark(tid: ThreadId, hint: usize) {
unsafe {
_lwp_unpark(tid, ptr::invalid(hint));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl Parker {
///
/// # Safety
/// The constructed parker must never be moved.
pub unsafe fn new(parker: *mut Parker) {
pub unsafe fn new_in_place(parker: *mut Parker) {
// Use the default mutex implementation to allow for simpler initialization.
// This could lead to undefined behaviour when deadlocking. This is avoided
// by not deadlocking. Note in particular the unlocking operation before any
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/windows/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ pub mod stdio;
pub mod thread;
pub mod thread_local_dtor;
pub mod thread_local_key;
pub mod thread_parker;
pub mod thread_parking;
pub mod time;
cfg_if::cfg_if! {
if #[cfg(not(target_vendor = "uwp"))] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ const NOTIFIED: i8 = 1;
impl Parker {
/// Construct the Windows parker. The UNIX parker implementation
/// requires this to happen in-place.
pub unsafe fn new(parker: *mut Parker) {
pub unsafe fn new_in_place(parker: *mut Parker) {
parker.write(Self { state: AtomicI8::new(EMPTY) });
}

Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys_common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub mod process;
pub mod thread;
pub mod thread_info;
pub mod thread_local_dtor;
pub mod thread_parker;
pub mod thread_parking;
pub mod wstr;
pub mod wtf8;

Expand Down
Loading

0 comments on commit ff3326d

Please sign in to comment.