From 5b712460017b708334b8f4e85065dbeb70d7de9f Mon Sep 17 00:00:00 2001 From: Yaroslav Volovich Date: Fri, 20 Feb 2026 17:52:21 +0000 Subject: [PATCH] fix: simplify macOS sleep inhibitor FFI (#12340) Summary - simplify the macOS sleep inhibitor FFI by replacing `dlopen` / `dlsym` / `transmute` with normal IOKit extern calls and `SAFETY` comments - switch to cfg-selected platform implementations (`imp::SleepInhibitor`) instead of `Box` - check in minimal IOKit bindings generated with `bindgen` and include them from the macOS backend - enable direct IOKit linkage in Bazel macOS builds by registering `IOKit` in the Bazel `osx.framework(...)` toolchain extension list - update `Cargo.lock` and `MODULE.bazel.lock` after removing the build-time `bindgen` dependency path Testing - `just fmt` - `cargo clippy -p codex-utils-sleep-inhibitor --all-targets -- -D warnings` - `cargo test -p codex-utils-sleep-inhibitor` - `bazel test //codex-rs/utils/sleep-inhibitor:all --test_output=errors` - `just bazel-lock-update` - `just bazel-lock-check` Context - follow-up to #11711 addressing Ryan's review comments - `bindgen` is used to generate the checked-in bindings file, but not at build time --- MODULE.bazel | 1 + codex-rs/Cargo.lock | 21 +- codex-rs/utils/sleep-inhibitor/Cargo.toml | 1 - codex-rs/utils/sleep-inhibitor/src/dummy.rs | 12 +- .../sleep-inhibitor/src/iokit_bindings.rs | 27 +++ codex-rs/utils/sleep-inhibitor/src/lib.rs | 25 +-- codex-rs/utils/sleep-inhibitor/src/macos.rs | 107 ++++++++++ .../sleep-inhibitor/src/macos_inhibitor.rs | 192 ------------------ 8 files changed, 159 insertions(+), 227 deletions(-) create mode 100644 codex-rs/utils/sleep-inhibitor/src/iokit_bindings.rs create mode 100644 codex-rs/utils/sleep-inhibitor/src/macos.rs delete mode 100644 codex-rs/utils/sleep-inhibitor/src/macos_inhibitor.rs diff --git a/MODULE.bazel b/MODULE.bazel index 35dd57500..821e41878 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -20,6 +20,7 @@ osx.framework(name = "CFNetwork") osx.framework(name = "FontServices") osx.framework(name = "Foundation") osx.framework(name = "ImageIO") +osx.framework(name = "IOKit") osx.framework(name = "Kernel") osx.framework(name = "OSLog") osx.framework(name = "Security") diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 20099b02e..8d7edc467 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -423,7 +423,7 @@ dependencies = [ "objc2-foundation", "parking_lot", "percent-encoding", - "windows-sys 0.52.0", + "windows-sys 0.60.2", "wl-clipboard-rs", "x11rb", ] @@ -2451,7 +2451,6 @@ name = "codex-utils-sleep-inhibitor" version = "0.0.0" dependencies = [ "core-foundation 0.9.4", - "libc", "tracing", ] @@ -3241,7 +3240,7 @@ dependencies = [ "libc", "option-ext", "redox_users 0.5.2", - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -3486,7 +3485,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -4879,7 +4878,7 @@ checksum = "3640c1c38b8e4e43584d8df18be5fc6b0aa314ce6ebf51b53313d4306cca8e46" dependencies = [ "hermit-abi", "libc", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -5587,7 +5586,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.61.2", ] [[package]] @@ -6153,7 +6152,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7d8fae84b431384b68627d0f9b3b1245fcf9f46f6c0e3dc902e9dce64edd1967" dependencies = [ "libc", - "windows-sys 0.45.0", + "windows-sys 0.61.2", ] [[package]] @@ -6714,7 +6713,7 @@ dependencies = [ "once_cell", "socket2 0.6.2", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.60.2", ] [[package]] @@ -7540,7 +7539,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.11.0", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -8917,7 +8916,7 @@ dependencies = [ "getrandom 0.3.4", "once_cell", "rustix 1.1.3", - "windows-sys 0.52.0", + "windows-sys 0.61.2", ] [[package]] @@ -10277,7 +10276,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.48.0", + "windows-sys 0.61.2", ] [[package]] diff --git a/codex-rs/utils/sleep-inhibitor/Cargo.toml b/codex-rs/utils/sleep-inhibitor/Cargo.toml index 703433d81..f691b191e 100644 --- a/codex-rs/utils/sleep-inhibitor/Cargo.toml +++ b/codex-rs/utils/sleep-inhibitor/Cargo.toml @@ -9,5 +9,4 @@ workspace = true [target.'cfg(target_os = "macos")'.dependencies] core-foundation = "0.9" -libc = { workspace = true } tracing = { workspace = true } diff --git a/codex-rs/utils/sleep-inhibitor/src/dummy.rs b/codex-rs/utils/sleep-inhibitor/src/dummy.rs index 767091ea9..a0be0dc65 100644 --- a/codex-rs/utils/sleep-inhibitor/src/dummy.rs +++ b/codex-rs/utils/sleep-inhibitor/src/dummy.rs @@ -1,16 +1,12 @@ -use crate::PlatformSleepInhibitor; - #[derive(Debug, Default)] -pub(crate) struct DummySleepInhibitor; +pub(crate) struct SleepInhibitor; -impl DummySleepInhibitor { +impl SleepInhibitor { pub(crate) fn new() -> Self { Self } -} -impl PlatformSleepInhibitor for DummySleepInhibitor { - fn acquire(&mut self) {} + pub(crate) fn acquire(&mut self) {} - fn release(&mut self) {} + pub(crate) fn release(&mut self) {} } diff --git a/codex-rs/utils/sleep-inhibitor/src/iokit_bindings.rs b/codex-rs/utils/sleep-inhibitor/src/iokit_bindings.rs new file mode 100644 index 000000000..50fd7fe51 --- /dev/null +++ b/codex-rs/utils/sleep-inhibitor/src/iokit_bindings.rs @@ -0,0 +1,27 @@ +/* automatically generated by rust-bindgen 0.71.1 */ + +pub const kIOReturnSuccess: u32 = 0; +#[repr(C)] +#[derive(Debug, Copy, Clone)] +pub struct __CFString { + _unused: [u8; 0], +} +pub type CFStringRef = *const __CFString; +pub type kern_return_t = ::std::os::raw::c_int; +pub type IOReturn = kern_return_t; +pub type IOPMAssertionID = u32; +pub type IOPMAssertionLevel = u32; +pub const kIOPMAssertionLevelOff: _bindgen_ty_36 = 0; +pub const kIOPMAssertionLevelOn: _bindgen_ty_36 = 255; +pub type _bindgen_ty_36 = ::std::os::raw::c_uint; +unsafe extern "C" { + pub fn IOPMAssertionRelease(AssertionID: IOPMAssertionID) -> IOReturn; +} +unsafe extern "C" { + pub fn IOPMAssertionCreateWithName( + AssertionType: CFStringRef, + AssertionLevel: IOPMAssertionLevel, + AssertionName: CFStringRef, + AssertionID: *mut IOPMAssertionID, + ) -> IOReturn; +} diff --git a/codex-rs/utils/sleep-inhibitor/src/lib.rs b/codex-rs/utils/sleep-inhibitor/src/lib.rs index 0f2f32df6..810fb170a 100644 --- a/codex-rs/utils/sleep-inhibitor/src/lib.rs +++ b/codex-rs/utils/sleep-inhibitor/src/lib.rs @@ -6,31 +6,26 @@ #[cfg(not(target_os = "macos"))] mod dummy; #[cfg(target_os = "macos")] -mod macos_inhibitor; +mod macos; -use std::fmt::Debug; +#[cfg(not(target_os = "macos"))] +use dummy as imp; +#[cfg(target_os = "macos")] +use macos as imp; /// Keeps the machine awake while a turn is in progress when enabled. #[derive(Debug)] pub struct SleepInhibitor { enabled: bool, - platform: Box, -} - -pub(crate) trait PlatformSleepInhibitor: Debug { - fn acquire(&mut self); - fn release(&mut self); + platform: imp::SleepInhibitor, } impl SleepInhibitor { pub fn new(enabled: bool) -> Self { - #[cfg(target_os = "macos")] - let platform: Box = - Box::new(macos_inhibitor::MacOsSleepInhibitor::new()); - #[cfg(not(target_os = "macos"))] - let platform: Box = Box::new(dummy::DummySleepInhibitor::new()); - - Self { enabled, platform } + Self { + enabled, + platform: imp::SleepInhibitor::new(), + } } /// Update the active turn state; turns sleep prevention on/off as needed. diff --git a/codex-rs/utils/sleep-inhibitor/src/macos.rs b/codex-rs/utils/sleep-inhibitor/src/macos.rs new file mode 100644 index 000000000..b8aaf29ab --- /dev/null +++ b/codex-rs/utils/sleep-inhibitor/src/macos.rs @@ -0,0 +1,107 @@ +use core_foundation::base::TCFType; +use core_foundation::string::CFString; +use tracing::warn; + +#[allow( + dead_code, + non_camel_case_types, + non_snake_case, + non_upper_case_globals, + clippy::all +)] +mod iokit { + #[link(name = "IOKit", kind = "framework")] + unsafe extern "C" {} + + include!("iokit_bindings.rs"); +} + +type IOPMAssertionID = iokit::IOPMAssertionID; +type IOPMAssertionLevel = iokit::IOPMAssertionLevel; +type IOReturn = iokit::IOReturn; + +const ASSERTION_REASON: &str = "Codex is running an active turn"; +// Apple exposes this assertion type as a `CFSTR(...)` macro, so bindgen cannot generate a +// reusable `CFStringRef` constant for it. +const ASSERTION_TYPE_PREVENT_USER_IDLE_SYSTEM_SLEEP: &str = "PreventUserIdleSystemSleep"; + +#[derive(Debug, Default)] +pub(crate) struct SleepInhibitor { + assertion: Option, +} + +impl SleepInhibitor { + pub(crate) fn new() -> Self { + Self::default() + } + + pub(crate) fn acquire(&mut self) { + if self.assertion.is_some() { + return; + } + + match MacSleepAssertion::create(ASSERTION_REASON) { + Ok(assertion) => { + self.assertion = Some(assertion); + } + Err(error) => { + warn!( + iokit_error = error, + "Failed to create macOS sleep-prevention assertion" + ); + } + } + } + + pub(crate) fn release(&mut self) { + self.assertion = None; + } +} + +#[derive(Debug)] +struct MacSleepAssertion { + id: IOPMAssertionID, +} + +impl MacSleepAssertion { + fn create(name: &str) -> Result { + let assertion_type = CFString::new(ASSERTION_TYPE_PREVENT_USER_IDLE_SYSTEM_SLEEP); + let assertion_name = CFString::new(name); + let mut id: IOPMAssertionID = 0; + // `core-foundation` and the generated bindings each declare an opaque `__CFString` type, + // so we cast to the bindgen pointer aliases before crossing the FFI boundary. + let assertion_type_ref: iokit::CFStringRef = assertion_type.as_concrete_TypeRef().cast(); + let assertion_name_ref: iokit::CFStringRef = assertion_name.as_concrete_TypeRef().cast(); + let result = unsafe { + // SAFETY: `assertion_type_ref` and `assertion_name_ref` are valid `CFStringRef`s and + // `&mut id` is a valid out-pointer for `IOPMAssertionCreateWithName` to initialize. + iokit::IOPMAssertionCreateWithName( + assertion_type_ref, + iokit::kIOPMAssertionLevelOn as IOPMAssertionLevel, + assertion_name_ref, + &mut id, + ) + }; + if result == iokit::kIOReturnSuccess as IOReturn { + Ok(Self { id }) + } else { + Err(result) + } + } +} + +impl Drop for MacSleepAssertion { + fn drop(&mut self) { + let result = unsafe { + // SAFETY: `self.id` was returned by `IOPMAssertionCreateWithName` and this `Drop` + // implementation releases it exactly once when the owning assertion is dropped. + iokit::IOPMAssertionRelease(self.id) + }; + if result != iokit::kIOReturnSuccess as IOReturn { + warn!( + iokit_error = result, + "Failed to release macOS sleep-prevention assertion" + ); + } + } +} diff --git a/codex-rs/utils/sleep-inhibitor/src/macos_inhibitor.rs b/codex-rs/utils/sleep-inhibitor/src/macos_inhibitor.rs deleted file mode 100644 index e788705e1..000000000 --- a/codex-rs/utils/sleep-inhibitor/src/macos_inhibitor.rs +++ /dev/null @@ -1,192 +0,0 @@ -use crate::PlatformSleepInhibitor; -use core_foundation::base::TCFType; -use core_foundation::string::CFString; -use core_foundation::string::CFStringRef; -use std::fmt::Debug; -use std::sync::OnceLock; -use tracing::warn; - -const ASSERTION_REASON: &str = "Codex is running an active turn"; -const MACOS_IDLE_SLEEP_ASSERTION_TYPE: &str = "PreventUserIdleSystemSleep"; -const IOKIT_FRAMEWORK_BINARY: &[u8] = b"/System/Library/Frameworks/IOKit.framework/IOKit\0"; -const IOPM_ASSERTION_CREATE_WITH_NAME_SYMBOL: &[u8] = b"IOPMAssertionCreateWithName\0"; -const IOPM_ASSERTION_RELEASE_SYMBOL: &[u8] = b"IOPMAssertionRelease\0"; -const IOKIT_ASSERTION_API_UNAVAILABLE: &str = "IOKit power assertion APIs are unavailable"; - -type IOPMAssertionReleaseFn = unsafe extern "C" fn(assertion_id: IOPMAssertionID) -> IOReturn; -type IOPMAssertionID = u32; -type IOPMAssertionLevel = u32; -type IOReturn = i32; - -const K_IOPM_ASSERTION_LEVEL_ON: IOPMAssertionLevel = 255; -const K_IORETURN_SUCCESS: IOReturn = 0; - -#[derive(Debug, Default)] -pub(crate) struct MacOsSleepInhibitor { - assertion: Option, -} - -impl MacOsSleepInhibitor { - pub(crate) fn new() -> Self { - Self { - ..Default::default() - } - } -} - -impl PlatformSleepInhibitor for MacOsSleepInhibitor { - fn acquire(&mut self) { - if self.assertion.is_some() { - return; - } - - match MacSleepAssertion::create(ASSERTION_REASON) { - Ok(assertion) => { - self.assertion = Some(assertion); - } - Err(error) => match error { - MacSleepAssertionError::ApiUnavailable(reason) => { - warn!(reason, "Failed to create macOS sleep-prevention assertion"); - } - MacSleepAssertionError::Iokit(code) => { - warn!( - iokit_error = code, - "Failed to create macOS sleep-prevention assertion" - ); - } - }, - } - } - - fn release(&mut self) { - self.assertion = None; - } -} - -#[derive(Debug)] -struct MacSleepAssertion { - id: IOPMAssertionID, -} - -impl MacSleepAssertion { - fn create(name: &str) -> Result { - let Some(api) = MacSleepApi::get() else { - return Err(MacSleepAssertionError::ApiUnavailable( - IOKIT_ASSERTION_API_UNAVAILABLE, - )); - }; - - let assertion_type = CFString::new(MACOS_IDLE_SLEEP_ASSERTION_TYPE); - let assertion_name = CFString::new(name); - let mut id: IOPMAssertionID = 0; - let result = unsafe { - (api.create_with_name)( - assertion_type.as_concrete_TypeRef(), - K_IOPM_ASSERTION_LEVEL_ON, - assertion_name.as_concrete_TypeRef(), - &mut id, - ) - }; - if result == K_IORETURN_SUCCESS { - Ok(Self { id }) - } else { - Err(MacSleepAssertionError::Iokit(result)) - } - } -} - -impl Drop for MacSleepAssertion { - fn drop(&mut self) { - let Some(api) = MacSleepApi::get() else { - warn!( - reason = IOKIT_ASSERTION_API_UNAVAILABLE, - "Failed to release macOS sleep-prevention assertion" - ); - return; - }; - - let result = unsafe { (api.release)(self.id) }; - if result != K_IORETURN_SUCCESS { - warn!( - iokit_error = result, - "Failed to release macOS sleep-prevention assertion" - ); - } - } -} - -#[derive(Debug, Clone, Copy)] -enum MacSleepAssertionError { - ApiUnavailable(&'static str), - Iokit(IOReturn), -} - -type IOPMAssertionCreateWithNameFn = unsafe extern "C" fn( - assertion_type: CFStringRef, - assertion_level: IOPMAssertionLevel, - assertion_name: CFStringRef, - assertion_id: *mut IOPMAssertionID, -) -> IOReturn; - -struct MacSleepApi { - // Keep the dlopen handle alive for the lifetime of the loaded symbols. - // This prevents accidental dlclose while function pointers are in use. - _iokit_handle: usize, - create_with_name: IOPMAssertionCreateWithNameFn, - release: IOPMAssertionReleaseFn, -} - -impl MacSleepApi { - fn get() -> Option<&'static Self> { - static API: OnceLock> = OnceLock::new(); - API.get_or_init(Self::load).as_ref() - } - - fn load() -> Option { - let handle = unsafe { - libc::dlopen( - IOKIT_FRAMEWORK_BINARY.as_ptr().cast(), - libc::RTLD_LOCAL | libc::RTLD_LAZY, - ) - }; - if handle.is_null() { - warn!(framework = "IOKit", "Failed to open IOKit framework"); - return None; - } - - let create_with_name = unsafe { - libc::dlsym( - handle, - IOPM_ASSERTION_CREATE_WITH_NAME_SYMBOL.as_ptr().cast(), - ) - }; - if create_with_name.is_null() { - warn!( - symbol = "IOPMAssertionCreateWithName", - "Failed to load IOKit symbol" - ); - let _ = unsafe { libc::dlclose(handle) }; - return None; - } - - let release = unsafe { libc::dlsym(handle, IOPM_ASSERTION_RELEASE_SYMBOL.as_ptr().cast()) }; - if release.is_null() { - warn!( - symbol = "IOPMAssertionRelease", - "Failed to load IOKit symbol" - ); - let _ = unsafe { libc::dlclose(handle) }; - return None; - } - - let create_with_name: IOPMAssertionCreateWithNameFn = - unsafe { std::mem::transmute(create_with_name) }; - let release: IOPMAssertionReleaseFn = unsafe { std::mem::transmute(release) }; - - Some(Self { - _iokit_handle: handle as usize, - create_with_name, - release, - }) - } -}