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

Use futex-based synchronization on Apple platforms #122408

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
8 changes: 4 additions & 4 deletions library/std/src/sys/pal/unix/fd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,9 @@ impl FileDesc {
super::weak::weak!(fn preadv(libc::c_int, *const libc::iovec, libc::c_int, off64_t) -> isize);

match preadv.get() {
Some(preadv) => {
Some(read) => {
let ret = cvt(unsafe {
preadv(
read(
self.as_raw_fd(),
bufs.as_mut_ptr() as *mut libc::iovec as *const libc::iovec,
cmp::min(bufs.len(), max_iov()) as libc::c_int,
Expand Down Expand Up @@ -477,9 +477,9 @@ impl FileDesc {
super::weak::weak!(fn pwritev(libc::c_int, *const libc::iovec, libc::c_int, off64_t) -> isize);

match pwritev.get() {
Some(pwritev) => {
Some(read) => {
let ret = cvt(unsafe {
pwritev(
read(
self.as_raw_fd(),
bufs.as_ptr() as *const libc::iovec,
cmp::min(bufs.len(), max_iov()) as libc::c_int,
Expand Down
184 changes: 184 additions & 0 deletions library/std/src/sys/pal/unix/futex.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#![cfg(any(
target_os = "linux",
target_vendor = "apple",
target_os = "android",
all(target_os = "emscripten", target_feature = "atomics"),
target_os = "freebsd",
Expand Down Expand Up @@ -145,6 +146,189 @@ pub fn futex_wake_all(futex: &AtomicU32) {
};
}

/// With macOS version 14.4, Apple introduced a public futex API. Unfortunately,
/// our minimum supported version is 10.12, so we need a fallback API. Luckily
/// for us, the underlying syscalls have been available since exactly that
/// version, so we just use those when needed. This is private API however,
/// which means we need to take care to avoid breakage if the syscall is removed
/// and to avoid apps being rejected from the App Store. To do this, we use weak
/// linkage emulation for both the public and the private API.
///
/// See https://developer.apple.com/documentation/os/os_sync_wait_on_address?language=objc
/// for documentation of the public API and
/// https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9f69dbd3c8c3fd30a/bsd/sys/ulock.h#L69
/// for the header file of the private API, along with its usage in libpthread
/// https://github.com/apple-oss-distributions/libpthread/blob/d8c4e3c212553d3e0f5d76bb7d45a8acd61302dc/src/pthread_cond.c#L463
#[cfg(target_vendor = "apple")]
mod apple {
use crate::ffi::{c_int, c_void};
// Explicitly use `dlsym` for the private fallback API to ensure that the
// App Store checks do not flag Apps using Rust as relying on private API.
use crate::sys::pal::weak::{dlsym, weak};

pub const OS_CLOCK_MACH_ABSOLUTE_TIME: u32 = 32;
pub const OS_SYNC_WAIT_ON_ADDRESS_NONE: u32 = 0;
pub const OS_SYNC_WAKE_BY_ADDRESS_NONE: u32 = 0;

pub const UL_COMPARE_AND_WAIT: u32 = 1;
pub const ULF_WAKE_ALL: u32 = 0x100;
// The syscalls support directly returning errors instead of going through errno.
pub const ULF_NO_ERRNO: u32 = 0x1000000;

// These functions appeared with macOS 14.4, iOS 17.4, tvOS 17.4, watchOS 10.4, visionOS 1.1.
weak! {
pub fn os_sync_wait_on_address(*mut c_void, u64, usize, u32) -> c_int
}

weak! {
pub fn os_sync_wait_on_address_with_timeout(*mut c_void, u64, usize, u32, u32, u64) -> c_int
}

weak! {
pub fn os_sync_wake_by_address_any(*mut c_void, usize, u32) -> c_int
}

weak! {
pub fn os_sync_wake_by_address_all(*mut c_void, usize, u32) -> c_int
}

// This syscall appeared with macOS 11.0.
// It is used to support nanosecond precision for timeouts, among other features.
dlsym! {
pub fn __ulock_wait2(u32, *mut c_void, u64, u64, u64) -> c_int
}

// These syscalls appeared with macOS 10.12.
dlsym! {
pub fn __ulock_wait(u32, *mut c_void, u64, u32) -> c_int
Copy link
Contributor

Choose a reason for hiding this comment

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

Mind explaining more what the thought process of these going undetected is? Seems like a risky chance that might require a hot fix if the MAS or iOS app store scanners see through this.

In reality I think this won't be enough and we need to call ulock things via syscall directly instead. This means that the "fallback" Apple futex implementation is only going to be usable on macOS.

I know Electron apps on the MAS use direct syscalls to emulate private functions, so this is probably what Rust should do too on macOS with precedence:

#define	SYS_ulock_wait     515
#define	SYS_ulock_wake     516
#define	SYS_ulock_wait2    544

For iOS, detect if the os_sync_ functions are available or fallback to the non-futex implementation :/ Calling a syscall directly is forbidden per the iOS headers so std can't do the same as macOS:

__WATCHOS_PROHIBITED __TVOS_PROHIBITED
__OS_AVAILABILITY_MSG(ios,deprecated=10.0,"syscall(2) is unsupported; "
    "please switch to a supported interface. For SYS_kdebug_trace use kdebug_signpost().")
__OS_AVAILABILITY_MSG(macosx,deprecated=10.12,"syscall(2) is unsupported; "
    "please switch to a supported interface. For SYS_kdebug_trace use kdebug_signpost().")
int	 syscall(int, ...);

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a "spirit of the law" kind of situation. Apple understandably wants App Store software to continue working in newer versions, so they prohibit private API use, as they want to be able to remove or change that API at will. By only using the private API as fallback and not linking it directly, we fulfil this wish. If we directly linked the private API instead, we'd get dynamic linker errors if they remove it, so we can't do that.

On the other hand, we break the "letter of the law" as we are using private API. In my opinion, this is totally fine and justified, but of course, this is only my interpretation. It would be great if someone from Apple could look over this, just so that we know that they are aware of this. My past attempt at reaching out to them for this has been unsuccessful.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you consider the solution of atomic-wait? They use libc++.

https://github.com/m-ou-se/atomic-wait/blob/main/src/macos.rs

Copy link
Member Author

Choose a reason for hiding this comment

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

But these only exist starting with macOS 11, which is way above our minimum of 10.12, so the ulock fallback would be needed regardless.

Copy link
Contributor

@madsmtm madsmtm Sep 26, 2024

Choose a reason for hiding this comment

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

This sets precedent for using private APIs in the standard library (our current use of weak! is only used for symbols that are available in newer versions), which I think is a fairly big deal, and should perhaps be discussed more broadly (in a separate issue? (that could be FCP'ed?)).

Somewhat related is #114186. Tagging in particular @thomcc and @workingjubilee, as they seem to have been involved in this kind of stuff before?


At the very least, we should use the dlsym! macro explicitly here, to make it very clear that we're not weakly linking the symbol (which would be very visible in the binary), but actually loading it at runtime, and thereby evading the App Store's checks (dlsym itself seems to be allowed).

I believe this is sufficient such that we do not need to do raw syscalls (which I remember to have caused problems for Go in the past since as the ABI isn't stable (?)).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we shouldn't be calling this directly, it'll trip the app store static analysis.

}

dlsym! {
pub fn __ulock_wake(u32, *mut c_void, u64) -> c_int
}
}

#[cfg(target_vendor = "apple")]
pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option<Duration>) -> bool {
use apple::*;

use crate::mem::size_of;

let addr = futex.as_ptr().cast();
let value = expected as u64;
let size = size_of::<u32>();
if let Some(timeout) = timeout {
let timeout_ns = timeout.as_nanos().clamp(1, u64::MAX as u128) as u64;
let timeout_ms = timeout.as_micros().clamp(1, u32::MAX as u128) as u32;

if let Some(wait) = os_sync_wait_on_address_with_timeout.get() {
let r = unsafe {
wait(
addr,
value,
size,
OS_SYNC_WAIT_ON_ADDRESS_NONE,
OS_CLOCK_MACH_ABSOLUTE_TIME,
timeout_ns,
)
};

// We promote spurious wakeups (reported as EINTR) to normal ones for
// simplicity.
r != -1 || super::os::errno() != libc::ETIMEDOUT
} else if let Some(wait) = __ulock_wait2.get() {
unsafe {
wait(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, addr, value, timeout_ns, 0)
!= -libc::ETIMEDOUT
}
} else if let Some(wait) = __ulock_wait.get() {
unsafe {
wait(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, addr, value, timeout_ms)
!= -libc::ETIMEDOUT
}
} else {
panic!("your system is below the minimum supported version of Rust");
}
} else {
if let Some(wait) = os_sync_wait_on_address.get() {
unsafe {
wait(addr, value, size, OS_SYNC_WAIT_ON_ADDRESS_NONE);
}
} else if let Some(wait) = __ulock_wait.get() {
unsafe {
wait(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, addr, value, 0);
}
} else {
panic!("your system is below the minimum supported version of Rust");
Copy link
Contributor

Choose a reason for hiding this comment

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

These panics also get to be deleted if raw syscalls are used :)

Copy link
Contributor

@madsmtm madsmtm Sep 26, 2024

Choose a reason for hiding this comment

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

If we do keep the panics, I'd go for an abort here instead, this is not recoverable in any way and avoiding the overhead from unwind landing pads would be nice.

}
true
}
}

#[cfg(target_vendor = "apple")]
pub fn futex_wake(futex: &AtomicU32) -> bool {
use apple::*;

use crate::io::Error;
use crate::mem::size_of;

let addr = futex.as_ptr().cast();
if let Some(wake) = os_sync_wake_by_address_any.get() {
unsafe { wake(addr, size_of::<u32>(), OS_SYNC_WAKE_BY_ADDRESS_NONE) == 0 }
} else if let Some(wake) = __ulock_wake.get() {
// Judging by its use in pthreads, __ulock_wake can get interrupted, so
// retry until either waking up a waiter or failing because there are no
// waiters (ENOENT).
loop {
let r = unsafe { wake(UL_COMPARE_AND_WAIT | ULF_NO_ERRNO, addr, 0) };

if r >= 0 {
return true;
} else {
match -r {
libc::ENOENT => return false,
libc::EINTR => continue,
err => panic!("__ulock_wake failed: {}", Error::from_raw_os_error(err)),
}
}
}
} else {
panic!("your system is below the minimum supported version of Rust");
}
}

#[cfg(target_vendor = "apple")]
pub fn futex_wake_all(futex: &AtomicU32) {
use apple::*;

use crate::io::Error;
use crate::mem::size_of;

let addr = futex.as_ptr().cast();

if let Some(wake) = os_sync_wake_by_address_all.get() {
unsafe {
wake(addr, size_of::<u32>(), OS_SYNC_WAKE_BY_ADDRESS_NONE);
}
} else if let Some(wake) = __ulock_wake.get() {
loop {
let r = unsafe { wake(UL_COMPARE_AND_WAIT | ULF_WAKE_ALL | ULF_NO_ERRNO, addr, 0) };

if r >= 0 {
return;
} else {
match -r {
libc::ENOENT => return,
libc::EINTR => continue,
err => panic!("__ulock_wake failed: {}", Error::from_raw_os_error(err)),
}
}
}
} else {
panic!("your system is below the minimum supported version of Rust");
}
}

#[cfg(target_os = "openbsd")]
pub fn futex_wait(futex: &AtomicU32, expected: u32, timeout: Option<Duration>) -> bool {
use super::time::Timespec;
Expand Down
2 changes: 2 additions & 0 deletions library/std/src/sys/pal/unix/sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
)))]
#![forbid(unsafe_op_in_unsafe_fn)]

#[cfg(not(target_vendor = "apple"))]
mod condvar;
mod mutex;

#[cfg(not(target_vendor = "apple"))]
pub use condvar::Condvar;
pub use mutex::Mutex;
11 changes: 6 additions & 5 deletions library/std/src/sys/pal/unix/weak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,16 @@ impl<F: Copy> ExternWeak<F> {
}

pub(crate) macro dlsym {
(fn $name:ident($($t:ty),*) -> $ret:ty) => (
dlsym!(fn $name($($t),*) -> $ret, stringify!($name));
($v:vis fn $name:ident($($t:ty),*) -> $ret:ty) => (
dlsym!($v fn $name($($t),*) -> $ret, stringify!($name));
),
(fn $name:ident($($t:ty),*) -> $ret:ty, $sym:expr) => (
static DLSYM: DlsymWeak<unsafe extern "C" fn($($t),*) -> $ret> =
($v:vis fn $name:ident($($t:ty),*) -> $ret:ty, $sym:expr) => (
#[allow(non_upper_case_globals)]
$v static $name: DlsymWeak<unsafe extern "C" fn($($t),*) -> $ret> =
DlsymWeak::new(concat!($sym, '\0'));
let $name = &DLSYM;
)
}

pub(crate) struct DlsymWeak<F> {
name: &'static str,
func: AtomicPtr<libc::c_void>,
Expand Down
1 change: 1 addition & 0 deletions library/std/src/sys/sync/condvar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ cfg_if::cfg_if! {
if #[cfg(any(
all(target_os = "windows", not(target_vendor="win7")),
target_os = "linux",
target_vendor = "apple",
target_os = "android",
target_os = "freebsd",
target_os = "openbsd",
Expand Down
4 changes: 4 additions & 0 deletions library/std/src/sys/sync/once/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ cfg_if::cfg_if! {
target_os = "dragonfly",
target_os = "fuchsia",
target_os = "hermit",
target_os = "macos",
target_os = "ios",
target_os = "tvos",
target_os = "watchos",
))] {
mod futex;
pub use futex::{Once, OnceState};
Expand Down
1 change: 1 addition & 0 deletions library/std/src/sys/sync/rwlock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ cfg_if::cfg_if! {
if #[cfg(any(
all(target_os = "windows", not(target_vendor = "win7")),
target_os = "linux",
target_vendor = "apple",
target_os = "android",
target_os = "freebsd",
target_os = "openbsd",
Expand Down
Loading
Loading