Harden unsafe usage across Rust codebase

vdavid · vdavid · commit 541804c3e777 · 2026-04-03T12:46:29.000+02:00
- Replace `MainThreadMarker::new_unchecked()` with checked `::new().expect()` in `lib.rs` and `menu/macos.rs` — turns silent UB into a clear panic if ever called off the main thread
- Add `// SAFETY:` comments to `transmute` calls in `drag_image_detection.rs` documenting the expected ObjC method signatures
- Replace generic `SendWrapper&lt;T&gt;` with narrow `SendableCFRunLoop` in `fsevent-stream` — scopes the `unsafe impl Send` to exactly the type that needs it, with safety rationale in the doc comment
diff --git a/apps/desktop/src-tauri/src/drag_image_detection.rs b/apps/desktop/src-tauri/src/drag_image_detection.rs
@@ -184,6 +184,9 @@ fn emit_modifiers_forced() {
 unsafe fn call_original_entered(this: &AnyObject, cmd: Sel, drag_info: &AnyObject) -> usize {
     unsafe {
         if let Some(&original) = ORIGINAL_ENTERED_IMP.get() {
+            // SAFETY: The original IMP was obtained from `draggingEntered:` which has the ObjC
+            // signature `- (NSDragOperation)draggingEntered:(id<NSDraggingInfo>)sender`, matching
+            // the C ABI as `fn(&AnyObject, Sel, &AnyObject) -> usize`.
             let f =
                 std::mem::transmute::<Imp, unsafe extern "C-unwind" fn(&AnyObject, Sel, &AnyObject) -> usize>(original);
             f(this, cmd, drag_info)
@@ -197,6 +200,7 @@ unsafe fn call_original_entered(this: &AnyObject, cmd: Sel, drag_info: &AnyObjec
 unsafe fn call_original_updated(this: &AnyObject, cmd: Sel, drag_info: &AnyObject) -> usize {
     unsafe {
         if let Some(&original) = ORIGINAL_UPDATED_IMP.get() {
+            // SAFETY: Same as call_original_entered — `draggingUpdated:` has an identical signature.
             let f =
                 std::mem::transmute::<Imp, unsafe extern "C-unwind" fn(&AnyObject, Sel, &AnyObject) -> usize>(original);
             f(this, cmd, drag_info)
@@ -211,6 +215,8 @@ unsafe fn call_original_updated(this: &AnyObject, cmd: Sel, drag_info: &AnyObjec
 unsafe fn call_original_exited(this: &AnyObject, cmd: Sel, drag_info: &AnyObject) {
     unsafe {
         if let Some(&original) = ORIGINAL_EXITED_IMP.get() {
+            // SAFETY: Same as call_original_entered — `draggingExited:` has the same parameter
+            // signature but returns void instead of NSDragOperation.
             let f = std::mem::transmute::<Imp, unsafe extern "C-unwind" fn(&AnyObject, Sel, &AnyObject)>(original);
             f(this, cmd, drag_info)
         }
diff --git a/apps/desktop/src-tauri/src/lib.rs b/apps/desktop/src-tauri/src/lib.rs
@@ -150,8 +150,8 @@ fn send_native_clipboard_action(menu_id: &str) {
         _ => return,
     };
 
-    // Safety: we're on the main thread (called from on_menu_event which runs on the main thread).
-    let mtm = unsafe { objc2::MainThreadMarker::new_unchecked() };
+    let mtm = objc2::MainThreadMarker::new()
+        .expect("send_native_clipboard_action must be called from the main thread");
     let ns_app = NSApplication::sharedApplication(mtm);
 
     // sendAction:to:from: with nil `to` sends to the first responder, exactly like
diff --git a/apps/desktop/src-tauri/src/menu/macos.rs b/apps/desktop/src-tauri/src/menu/macos.rs
@@ -340,8 +340,8 @@ pub(crate) fn cleanup_macos_menus() {
 }
 
 fn cleanup_macos_menus_inner() {
-    // This runs during Tauri setup on the main thread.
-    let mtm = unsafe { MainThreadMarker::new_unchecked() };
+    let mtm = MainThreadMarker::new()
+        .expect("cleanup_macos_menus_inner must be called from the main thread");
     let app = NSApplication::sharedApplication(mtm);
     let Some(main_menu) = app.mainMenu() else {
         return;
@@ -413,7 +413,8 @@ pub(crate) fn set_macos_menu_icons() {
 }
 
 fn set_macos_menu_icons_inner() {
-    let mtm = unsafe { MainThreadMarker::new_unchecked() };
+    let mtm = MainThreadMarker::new()
+        .expect("set_macos_menu_icons_inner must be called from the main thread");
     let app = NSApplication::sharedApplication(mtm);
     let Some(main_menu) = app.mainMenu() else {
         return;
diff --git a/crates/fsevent-stream/src/stream.rs b/crates/fsevent-stream/src/stream.rs
@@ -175,15 +175,15 @@ pub(crate) struct StreamContextInfo {
 
 impl_release_callback!(release_context, StreamContextInfo);
 
-struct SendWrapper<T>(T);
-
-unsafe impl<T> Send for SendWrapper<T> {}
+/// A CFRunLoop that can be sent across threads.
+///
+/// SAFETY: CFRunLoop is a Core Foundation type. Apple documents that CF objects can be
+/// retained/released and used across threads. CFRunLoopStop (the only cross-thread operation
+/// we perform) is explicitly documented as thread-safe.
+/// https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/ThreadSafetySummary/ThreadSafetySummary.html
+struct SendableCFRunLoop(CFRunLoop);
 
-impl<T> SendWrapper<T> {
-    const unsafe fn new(t: T) -> Self {
-        Self(t)
-    }
-}
+unsafe impl Send for SendableCFRunLoop {}
 
 /// Create a new [`EventStream`](EventStream) and [`EventStreamHandler`](EventStreamHandler) pair.
 ///
@@ -265,11 +265,8 @@ pub fn create_event_stream<P: AsRef<Path>>(
         stream.start();
 
         // the calling to CFRunLoopRun will be terminated by CFRunLoopStop call in drop()
-        // Safety:
-        // - According to the Apple documentation, it's safe to move `CFRef`s across threads.
-        //   https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Multithreading/ThreadSafetySummary/ThreadSafetySummary.html
         runloop_tx
-            .send(unsafe { SendWrapper::new(current_runloop) })
+            .send(SendableCFRunLoop(current_runloop))
             .expect("send runloop to stream");
 
         CFRunLoop::run_current();
