From 8118d6a951fdd23a274d8a81e6b0a778ab69d541 Mon Sep 17 00:00:00 2001 From: Kevin Mehall Date: Sun, 3 Dec 2023 18:12:59 -0800 Subject: [PATCH 1/2] Make CFRunLoopMode a safe wrapper with a lifetime. Fixes https://github.com/servo/core-foundation-rs/issues/648 --- core-foundation/src/filedescriptor.rs | 7 +--- core-foundation/src/runloop.rs | 56 +++++++++++++++++---------- core-graphics/src/event.rs | 4 +- 3 files changed, 40 insertions(+), 27 deletions(-) diff --git a/core-foundation/src/filedescriptor.rs b/core-foundation/src/filedescriptor.rs index b45ff1c4..ad391eb6 100644 --- a/core-foundation/src/filedescriptor.rs +++ b/core-foundation/src/filedescriptor.rs @@ -98,10 +98,9 @@ impl AsRawFd for CFFileDescriptor { #[cfg(test)] mod test { use super::*; - use crate::runloop::CFRunLoop; + use crate::runloop::{CFRunLoop, CFRunLoopMode}; use core::ffi::c_void; use core_foundation_sys::base::CFOptionFlags; - use core_foundation_sys::runloop::kCFRunLoopDefaultMode; use libc::O_RDWR; use std::ffi::CString; @@ -155,9 +154,7 @@ mod test { let run_loop = CFRunLoop::get_current(); let source = CFRunLoopSource::from_file_descriptor(&cf_fd, 0); assert!(source.is_some()); - unsafe { - run_loop.add_source(&source.unwrap(), kCFRunLoopDefaultMode); - } + run_loop.add_source(&source.unwrap(), CFRunLoopMode::default()); info.value = 0; cf_fd.enable_callbacks(kCFFileDescriptorReadCallBack); diff --git a/core-foundation/src/runloop.rs b/core-foundation/src/runloop.rs index e3f1e264..fc6b7db5 100644 --- a/core-foundation/src/runloop.rs +++ b/core-foundation/src/runloop.rs @@ -9,6 +9,8 @@ #![allow(non_upper_case_globals)] +use std::marker::PhantomData; + use core_foundation_sys::base::CFIndex; use core_foundation_sys::base::{kCFAllocatorDefault, CFOptionFlags}; pub use core_foundation_sys::runloop::*; @@ -19,7 +21,23 @@ use crate::date::{CFAbsoluteTime, CFTimeInterval}; use crate::filedescriptor::CFFileDescriptor; use crate::string::CFString; -pub type CFRunLoopMode = CFStringRef; +pub struct CFRunLoopMode<'a>(CFStringRef, PhantomData<&'a CFString>); + +impl<'a> CFRunLoopMode<'a> { + pub fn new(s: &'a CFString) -> CFRunLoopMode<'a> { + CFRunLoopMode(s.as_concrete_TypeRef(), PhantomData) + } + + #[doc(alias = "kCFRunLoopCommonModes")] + pub fn common() -> CFRunLoopMode<'static> { + unsafe { CFRunLoopMode(kCFRunLoopCommonModes, PhantomData) } + } + + #[doc(alias = "kCFRunLoopDefaultMode")] + pub fn default() -> CFRunLoopMode<'static> { + unsafe { CFRunLoopMode(kCFRunLoopDefaultMode, PhantomData) } + } +} declare_TCFType!(CFRunLoop, CFRunLoopRef); impl_TCFType!(CFRunLoop, CFRunLoopRef, CFRunLoopGetTypeID); @@ -59,7 +77,7 @@ impl CFRunLoop { } pub fn run_in_mode( - mode: CFStringRef, + mode: CFRunLoopMode, duration: std::time::Duration, return_after_source_handled: bool, ) -> CFRunLoopRunResult { @@ -67,7 +85,7 @@ impl CFRunLoop { let return_after_source_handled = if return_after_source_handled { 1 } else { 0 }; unsafe { - match CFRunLoopRunInMode(mode, seconds, return_after_source_handled) { + match CFRunLoopRunInMode(mode.0, seconds, return_after_source_handled) { 2 => CFRunLoopRunResult::Stopped, 3 => CFRunLoopRunResult::TimedOut, 4 => CFRunLoopRunResult::HandledSource, @@ -95,50 +113,50 @@ impl CFRunLoop { } pub fn contains_timer(&self, timer: &CFRunLoopTimer, mode: CFRunLoopMode) -> bool { - unsafe { CFRunLoopContainsTimer(self.0, timer.0, mode) != 0 } + unsafe { CFRunLoopContainsTimer(self.0, timer.0, mode.0) != 0 } } pub fn add_timer(&self, timer: &CFRunLoopTimer, mode: CFRunLoopMode) { unsafe { - CFRunLoopAddTimer(self.0, timer.0, mode); + CFRunLoopAddTimer(self.0, timer.0, mode.0); } } pub fn remove_timer(&self, timer: &CFRunLoopTimer, mode: CFRunLoopMode) { unsafe { - CFRunLoopRemoveTimer(self.0, timer.0, mode); + CFRunLoopRemoveTimer(self.0, timer.0, mode.0); } } pub fn contains_source(&self, source: &CFRunLoopSource, mode: CFRunLoopMode) -> bool { - unsafe { CFRunLoopContainsSource(self.0, source.0, mode) != 0 } + unsafe { CFRunLoopContainsSource(self.0, source.0, mode.0) != 0 } } pub fn add_source(&self, source: &CFRunLoopSource, mode: CFRunLoopMode) { unsafe { - CFRunLoopAddSource(self.0, source.0, mode); + CFRunLoopAddSource(self.0, source.0, mode.0); } } pub fn remove_source(&self, source: &CFRunLoopSource, mode: CFRunLoopMode) { unsafe { - CFRunLoopRemoveSource(self.0, source.0, mode); + CFRunLoopRemoveSource(self.0, source.0, mode.0); } } pub fn contains_observer(&self, observer: &CFRunLoopObserver, mode: CFRunLoopMode) -> bool { - unsafe { CFRunLoopContainsObserver(self.0, observer.0, mode) != 0 } + unsafe { CFRunLoopContainsObserver(self.0, observer.0, mode.0) != 0 } } pub fn add_observer(&self, observer: &CFRunLoopObserver, mode: CFRunLoopMode) { unsafe { - CFRunLoopAddObserver(self.0, observer.0, mode); + CFRunLoopAddObserver(self.0, observer.0, mode.0); } } pub fn remove_observer(&self, observer: &CFRunLoopObserver, mode: CFRunLoopMode) { unsafe { - CFRunLoopRemoveObserver(self.0, observer.0, mode); + CFRunLoopRemoveObserver(self.0, observer.0, mode.0); } } } @@ -222,9 +240,8 @@ mod test { let run_loop_timer = CFRunLoopTimer::new(now + 0.20f64, 0f64, 0, 0, timer_popped, &mut context); - unsafe { - run_loop.add_timer(&run_loop_timer, kCFRunLoopDefaultMode); - } + run_loop.add_timer(&run_loop_timer, CFRunLoopMode::default()); + CFRunLoop::run_current(); let elapsed = elapsed_rx.try_recv().unwrap(); println!("wait_200_milliseconds, elapsed: {}", elapsed); @@ -277,7 +294,7 @@ mod test { }; let runloop = CFRunLoop::get_current(); - runloop.add_observer(&observer, unsafe { kCFRunLoopDefaultMode }); + runloop.add_observer(&observer, CFRunLoopMode::default()); let timer = CFRunLoopTimer::new( CFDate::now().abs_time() + 1f64, @@ -287,11 +304,10 @@ mod test { observe_timer_popped, null_mut(), ); - runloop.add_timer(&timer, unsafe { kCFRunLoopDefaultMode }); + runloop.add_timer(&timer, CFRunLoopMode::default()); - let result = unsafe { - CFRunLoop::run_in_mode(kCFRunLoopDefaultMode, Duration::from_secs(10), false) - }; + let result = + CFRunLoop::run_in_mode(CFRunLoopMode::default(), Duration::from_secs(10), false); assert_eq!(result, CFRunLoopRunResult::Stopped); diff --git a/core-graphics/src/event.rs b/core-graphics/src/event.rs index 625702d8..95295382 100644 --- a/core-graphics/src/event.rs +++ b/core-graphics/src/event.rs @@ -443,7 +443,7 @@ unsafe extern "C" fn cg_event_tap_callback_internal( } /// ```no_run -///use core_foundation::runloop::{kCFRunLoopCommonModes, CFRunLoop}; +///use core_foundation::runloop::{CFRunLoop, CFRunLoopMode}; ///use core_graphics::event::{CGEventTap, CGEventTapLocation, CGEventTapPlacement, CGEventTapOptions, CGEventType}; ///let current = CFRunLoop::get_current(); ///match CGEventTap::new( @@ -461,7 +461,7 @@ unsafe extern "C" fn cg_event_tap_callback_internal( /// .mach_port /// .create_runloop_source(0) /// .expect("Somethings is bad "); -/// current.add_source(&loop_source, kCFRunLoopCommonModes); +/// current.add_source(&loop_source, CFRunLoopMode::common()); /// tap.enable(); /// CFRunLoop::run_current(); /// }, From db98303dc365a17017f00f132a79f9f1448d2e6a Mon Sep 17 00:00:00 2001 From: Kevin Mehall Date: Tue, 13 Aug 2024 18:29:54 -0700 Subject: [PATCH 2/2] Rename CFRunLoopMode constructors for clippy::should_implement_trait lint --- core-foundation/src/filedescriptor.rs | 2 +- core-foundation/src/runloop.rs | 17 ++++++++++------- core-graphics/src/event.rs | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/core-foundation/src/filedescriptor.rs b/core-foundation/src/filedescriptor.rs index ad391eb6..985567fe 100644 --- a/core-foundation/src/filedescriptor.rs +++ b/core-foundation/src/filedescriptor.rs @@ -154,7 +154,7 @@ mod test { let run_loop = CFRunLoop::get_current(); let source = CFRunLoopSource::from_file_descriptor(&cf_fd, 0); assert!(source.is_some()); - run_loop.add_source(&source.unwrap(), CFRunLoopMode::default()); + run_loop.add_source(&source.unwrap(), CFRunLoopMode::default_mode()); info.value = 0; cf_fd.enable_callbacks(kCFFileDescriptorReadCallBack); diff --git a/core-foundation/src/runloop.rs b/core-foundation/src/runloop.rs index fc6b7db5..69f377f3 100644 --- a/core-foundation/src/runloop.rs +++ b/core-foundation/src/runloop.rs @@ -29,12 +29,12 @@ impl<'a> CFRunLoopMode<'a> { } #[doc(alias = "kCFRunLoopCommonModes")] - pub fn common() -> CFRunLoopMode<'static> { + pub fn common_modes() -> CFRunLoopMode<'static> { unsafe { CFRunLoopMode(kCFRunLoopCommonModes, PhantomData) } } #[doc(alias = "kCFRunLoopDefaultMode")] - pub fn default() -> CFRunLoopMode<'static> { + pub fn default_mode() -> CFRunLoopMode<'static> { unsafe { CFRunLoopMode(kCFRunLoopDefaultMode, PhantomData) } } } @@ -240,7 +240,7 @@ mod test { let run_loop_timer = CFRunLoopTimer::new(now + 0.20f64, 0f64, 0, 0, timer_popped, &mut context); - run_loop.add_timer(&run_loop_timer, CFRunLoopMode::default()); + run_loop.add_timer(&run_loop_timer, CFRunLoopMode::default_mode()); CFRunLoop::run_current(); let elapsed = elapsed_rx.try_recv().unwrap(); @@ -294,7 +294,7 @@ mod test { }; let runloop = CFRunLoop::get_current(); - runloop.add_observer(&observer, CFRunLoopMode::default()); + runloop.add_observer(&observer, CFRunLoopMode::default_mode()); let timer = CFRunLoopTimer::new( CFDate::now().abs_time() + 1f64, @@ -304,10 +304,13 @@ mod test { observe_timer_popped, null_mut(), ); - runloop.add_timer(&timer, CFRunLoopMode::default()); + runloop.add_timer(&timer, CFRunLoopMode::default_mode()); - let result = - CFRunLoop::run_in_mode(CFRunLoopMode::default(), Duration::from_secs(10), false); + let result = CFRunLoop::run_in_mode( + CFRunLoopMode::default_mode(), + Duration::from_secs(10), + false, + ); assert_eq!(result, CFRunLoopRunResult::Stopped); diff --git a/core-graphics/src/event.rs b/core-graphics/src/event.rs index 95295382..c0dc1350 100644 --- a/core-graphics/src/event.rs +++ b/core-graphics/src/event.rs @@ -461,7 +461,7 @@ unsafe extern "C" fn cg_event_tap_callback_internal( /// .mach_port /// .create_runloop_source(0) /// .expect("Somethings is bad "); -/// current.add_source(&loop_source, CFRunLoopMode::common()); +/// current.add_source(&loop_source, CFRunLoopMode::common_modes()); /// tap.enable(); /// CFRunLoop::run_current(); /// },