Skip to content

Commit

Permalink
Fix darwin ulock usage on macOS (#9545)
Browse files Browse the repository at this point in the history
* Fix darwin ulock usage on macOS

* Fix review comments, and correctly handle timeout overflow on darwin

* Apply more review suggestions
  • Loading branch information
thomcc authored Aug 14, 2021
1 parent 65208a7 commit c2635f9
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 10 deletions.
35 changes: 26 additions & 9 deletions lib/std/Thread/Futex.zig
Original file line number Diff line number Diff line change
Expand Up @@ -180,32 +180,48 @@ const DarwinFutex = struct {
const darwin = std.os.darwin;

fn wait(ptr: *const Atomic(u32), expect: u32, timeout: ?u64) error{TimedOut}!void {
// ulock_wait() uses micro-second timeouts, where 0 = INIFITE or no-timeout
var timeout_us: u32 = 0;
if (timeout) |timeout_ns| {
timeout_us = @intCast(u32, timeout_ns / std.time.ns_per_us);
}

// Darwin XNU 7195.50.7.100.1 introduced __ulock_wait2 and migrated code paths (notably pthread_cond_t) towards it:
// https://github.com/apple/darwin-xnu/commit/d4061fb0260b3ed486147341b72468f836ed6c8f#diff-08f993cc40af475663274687b7c326cc6c3031e0db3ac8de7b24624610616be6
//
// This XNU version appears to correspond to 11.0.1:
// https://kernelshaman.blogspot.com/2021/01/building-xnu-for-macos-big-sur-1101.html
//
// ulock_wait() uses 32-bit micro-second timeouts where 0 = INFINITE or no-timeout
// ulock_wait2() uses 64-bit nano-second timeouts (with the same convention)
var timeout_ns: u64 = 0;
if (timeout) |timeout_value| {
// This should be checked by the caller.
assert(timeout_value != 0);
timeout_ns = timeout_value;
}
const addr = @ptrCast(*const c_void, ptr);
const flags = darwin.UL_COMPARE_AND_WAIT | darwin.ULF_NO_ERRNO;
// If we're using `__ulock_wait` and `timeout` is too big to fit inside a `u32` count of
// micro-seconds (around 70min), we'll request a shorter timeout. This is fine (users
// should handle spurious wakeups), but we need to remember that we did so, so that
// we don't return `TimedOut` incorrectly. If that happens, we set this variable to
// true so that we we know to ignore the ETIMEDOUT result.
var timeout_overflowed = false;
const status = blk: {
if (target.os.version_range.semver.max.major >= 11) {
break :blk darwin.__ulock_wait2(flags, addr, expect, timeout_us, 0);
break :blk darwin.__ulock_wait2(flags, addr, expect, timeout_ns, 0);
} else {
const timeout_us = std.math.cast(u32, timeout_ns / std.time.ns_per_us) catch overflow: {
timeout_overflowed = true;
break :overflow std.math.maxInt(u32);
};
break :blk darwin.__ulock_wait(flags, addr, expect, timeout_us);
}
};

if (status >= 0) return;
switch (-status) {
darwin.EINTR => {},
darwin.EFAULT => unreachable,
darwin.ETIMEDOUT => return error.TimedOut,
// Address of the futex is paged out. This is unlikely, but possible in theory, and
// pthread/libdispatch on darwin bother to handle it. In this case we'll return
// without waiting, but the caller should retry anyway.
darwin.EFAULT => {},
darwin.ETIMEDOUT => if (!timeout_overflowed) return error.TimedOut,
else => unreachable,
}
}
Expand All @@ -223,6 +239,7 @@ const DarwinFutex = struct {
if (status >= 0) return;
switch (-status) {
darwin.EINTR => continue, // spurious wake()
darwin.EFAULT => continue, // address of the lock was paged out
darwin.ENOENT => return, // nothing was woken up
darwin.EALREADY => unreachable, // only for ULF_WAKE_THREAD
else => unreachable,
Expand Down
2 changes: 1 addition & 1 deletion lib/std/c/darwin.zig
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ pub const UL_COMPARE_AND_WAIT64 = 5;
pub const UL_COMPARE_AND_WAIT64_SHARED = 6;
pub const ULF_WAIT_ADAPTIVE_SPIN = 0x40000;

pub extern "c" fn __ulock_wait2(op: u32, addr: ?*const c_void, val: u64, timeout_us: u32, val2: u64) c_int;
pub extern "c" fn __ulock_wait2(op: u32, addr: ?*const c_void, val: u64, timeout_ns: u64, val2: u64) c_int;
pub extern "c" fn __ulock_wait(op: u32, addr: ?*const c_void, val: u64, timeout_us: u32) c_int;
pub extern "c" fn __ulock_wake(op: u32, addr: ?*const c_void, val: u64) c_int;

Expand Down

0 comments on commit c2635f9

Please sign in to comment.