Fix unsoundness in CGEventTap API

Author: tmandry

core-graphics/src/event.rs

@@ -6,7 +6,7 @@ use bitflags::bitflags;
 use core::ffi::{c_ulong, c_void};
 use core_foundation::{
     base::{CFRelease, CFRetain, CFTypeID, TCFType},
-    mach_port::{CFMachPort, CFMachPortRef},
+    mach_port::{CFMachPort, CFMachPortInvalidate, CFMachPortRef},
 };
 use foreign_types::{foreign_type, ForeignType};
 use std::mem::ManuallyDrop;
@@ -417,7 +417,7 @@ macro_rules! CGEventMaskBit {
 }
 
 pub type CGEventTapProxy = *const c_void;
-pub type CGEventTapCallBackFn<'tap_life> =
+type CGEventTapCallBackFn<'tap_life> =
     Box<dyn Fn(CGEventTapProxy, CGEventType, &CGEvent) -> Option<CGEvent> + 'tap_life>;
 type CGEventTapCallBackInternal = unsafe extern "C" fn(
     proxy: CGEventTapProxy,
@@ -458,9 +458,9 @@ unsafe extern "C" fn cg_event_tap_callback_internal(
 /// ) {
 ///     Ok(tap) => unsafe {
 ///         let loop_source = tap
-///             .mach_port
+///             .mach_port()
 ///             .create_runloop_source(0)
-///             .expect("Somethings is bad ");
+///             .expect("Runloop source creation failed");
 ///         current.add_source(&loop_source, kCFRunLoopCommonModes);
 ///         tap.enable();
 ///         CFRunLoop::run_current();
@@ -469,9 +469,8 @@ unsafe extern "C" fn cg_event_tap_callback_internal(
 /// }
 /// ```
 pub struct CGEventTap<'tap_life> {
-    pub mach_port: CFMachPort,
-    pub callback_ref:
-        Box<dyn Fn(CGEventTapProxy, CGEventType, &CGEvent) -> Option<CGEvent> + 'tap_life>,
+    mach_port: CFMachPort,
+    _callback: Box<CGEventTapCallBackFn<'tap_life>>,
 }
 
 impl<'tap_life> CGEventTap<'tap_life> {
@@ -487,7 +486,7 @@ impl<'tap_life> CGEventTap<'tap_life> {
             .fold(CGEventType::Null as CGEventMask, |mask, &etype| {
                 mask | CGEventMaskBit!(etype)
             });
-        let cb = Box::new(Box::new(callback) as CGEventTapCallBackFn);
+        let cb: Box<CGEventTapCallBackFn> = Box::new(Box::new(callback));
         let cbr = Box::into_raw(cb);
         unsafe {
             let event_tap_ref = CGEventTapCreate(
@@ -502,7 +501,7 @@ impl<'tap_life> CGEventTap<'tap_life> {
             if !event_tap_ref.is_null() {
                 Ok(Self {
                     mach_port: (CFMachPort::wrap_under_create_rule(event_tap_ref)),
-                    callback_ref: Box::from_raw(cbr),
+                    _callback: Box::from_raw(cbr),
                 })
             } else {
                 let _ = Box::from_raw(cbr);
@@ -511,11 +510,21 @@ impl<'tap_life> CGEventTap<'tap_life> {
         }
     }
 
+    pub fn mach_port(&self) -> &CFMachPort {
+        &self.mach_port
+    }
+
     pub fn enable(&self) {
         unsafe { CGEventTapEnable(self.mach_port.as_concrete_TypeRef(), true) }
     }
 }
 
+impl Drop for CGEventTap<'_> {
+    fn drop(&mut self) {
+        unsafe { CFMachPortInvalidate(self.mach_port.as_CFTypeRef() as *mut _) };
+    }
+}
+
 foreign_type! {
     #[doc(hidden)]
     pub unsafe type CGEvent {