Fix native file dialogs freezing the event loop (#440)

On macOS, fix native file dialogs hanging the event loop and having multiple windows would prevent run_return from ever returning. Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com> Co-authored-by: Kevin King <kcking@users.noreply.github.com> Co-authored-by: Emil Ernerfeldt <emil.ernerfeldt@gmail.com> Co-authored-by: Kevin King <kcking@users.noreply.github.com>
tauri-apps · Jun 22, 2022 · 5c9cc21 · 5c9cc21
1 parent 5a664fd
commit 5c9cc21
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 62 deletions.
diff --git a/.changes/dialog-freeze.md b/.changes/dialog-freeze.md
@@ -0,0 +1,6 @@
+---
+"tao": patch
+---
+
+On macOS, fix native file dialogs hanging the event loop and
+having multiple windows would prevent `run_return` from ever returning.
diff --git a/src/platform_impl/linux/event_loop.rs b/src/platform_impl/linux/event_loop.rs
@@ -17,11 +17,12 @@ use gio::{prelude::*, Cancellable};
 use glib::{source::Priority, Continue, MainContext};
 use gtk::{builders::AboutDialogBuilder, prelude::*, Inhibit};
 
-use crate::event::{MouseScrollDelta, TouchPhase};
 use crate::{
   accelerator::AcceleratorId,
   dpi::{LogicalPosition, LogicalSize},
-  event::{ElementState, Event, MouseButton, StartCause, WindowEvent},
+  event::{
+    ElementState, Event, MouseButton, MouseScrollDelta, StartCause, TouchPhase, WindowEvent,
+  },
   event_loop::{ControlFlow, EventLoopClosed, EventLoopWindowTarget as RootELW},
   keyboard::ModifiersState,
   menu::{MenuItem, MenuType},

diff --git a/src/platform_impl/macos/app_state.rs b/src/platform_impl/macos/app_state.rs
@@ -124,7 +124,6 @@ impl<T> EventHandler for EventLoopHandler<T> {
 struct Handler {
   ready: AtomicBool,
   in_callback: AtomicBool,
-  dialog_is_closing: AtomicBool,
   control_flow: Mutex<ControlFlow>,
   control_flow_prev: Mutex<ControlFlow>,
   start_time: Mutex<Option<Instant>>,
@@ -256,8 +255,6 @@ impl Handler {
   }
 }
 
-pub static INTERRUPT_EVENT_LOOP_EXIT: AtomicBool = AtomicBool::new(false);
-
 pub enum AppState {}
 
 impl AppState {
@@ -304,7 +301,8 @@ impl AppState {
     let panic_info = panic_info
       .upgrade()
       .expect("The panic info must exist here. This failure indicates a developer error.");
-    if panic_info.is_panicking() || !HANDLER.is_ready() {
+    // Return when in callback due to https://github.com/rust-windowing/winit/issues/1779
+    if panic_info.is_panicking() || !HANDLER.is_ready() || HANDLER.get_in_callback() {
       return;
     }
     let start = HANDLER.get_start_time().unwrap();
@@ -370,56 +368,29 @@ impl AppState {
     let panic_info = panic_info
       .upgrade()
       .expect("The panic info must exist here. This failure indicates a developer error.");
-    if panic_info.is_panicking() || !HANDLER.is_ready() {
+    // Return when in callback due to https://github.com/rust-windowing/winit/issues/1779
+    if panic_info.is_panicking() || !HANDLER.is_ready() || HANDLER.get_in_callback() {
       return;
     }
-    if !HANDLER.get_in_callback() {
-      HANDLER.set_in_callback(true);
-      HANDLER.handle_user_events();
-      for event in HANDLER.take_events() {
-        HANDLER.handle_nonuser_event(event);
-      }
-      HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::MainEventsCleared));
-      for window_id in HANDLER.should_redraw() {
-        HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::RedrawRequested(window_id)));
-      }
-      HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::RedrawEventsCleared));
-      HANDLER.set_in_callback(false);
+    HANDLER.set_in_callback(true);
+    HANDLER.handle_user_events();
+    for event in HANDLER.take_events() {
+      HANDLER.handle_nonuser_event(event);
     }
+    HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::MainEventsCleared));
+    for window_id in HANDLER.should_redraw() {
+      HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::RedrawRequested(window_id)));
+    }
+    HANDLER.handle_nonuser_event(EventWrapper::StaticEvent(Event::RedrawEventsCleared));
+    HANDLER.set_in_callback(false);
     if HANDLER.should_exit() {
       unsafe {
         let app: id = NSApp();
-        let windows: id = msg_send![app, windows];
-        let window_count: usize = msg_send![windows, count];
-
-        let dialog_open = if window_count > 1 {
-          let dialog: id = msg_send![windows, lastObject];
-          let is_main_window: BOOL = msg_send![dialog, isMainWindow];
-          let is_visible: BOOL = msg_send![dialog, isVisible];
-          is_visible != NO && is_main_window == NO
-        } else {
-          false
-        };
-
-        let dialog_is_closing = HANDLER.dialog_is_closing.load(Ordering::SeqCst);
         let pool = NSAutoreleasePool::new(nil);
-        if !INTERRUPT_EVENT_LOOP_EXIT.load(Ordering::SeqCst) && !dialog_open && !dialog_is_closing {
-          let () = msg_send![app, stop: nil];
-          // To stop event loop immediately, we need to post some event here.
-          post_dummy_event(app);
-        }
+        let () = msg_send![app, stop: nil];
+        // To stop event loop immediately, we need to post some event here.
+        post_dummy_event(app);
         pool.drain();
-
-        if window_count > 0 {
-          let window: id = msg_send![windows, firstObject];
-          let window_has_focus: BOOL = msg_send![window, isKeyWindow];
-          if !dialog_open && window_has_focus != NO && dialog_is_closing {
-            HANDLER.dialog_is_closing.store(false, Ordering::SeqCst);
-          }
-          if dialog_open {
-            HANDLER.dialog_is_closing.store(true, Ordering::SeqCst);
-          }
-        }
       };
     }
     HANDLER.update_start_time();

diff --git a/src/platform_impl/macos/window.rs b/src/platform_impl/macos/window.rs
@@ -22,7 +22,7 @@ use crate::{
   monitor::{MonitorHandle as RootMonitorHandle, VideoMode as RootVideoMode},
   platform::macos::WindowExtMacOS,
   platform_impl::platform::{
-    app_state::{AppState, INTERRUPT_EVENT_LOOP_EXIT},
+    app_state::AppState,
     ffi, menu,
     monitor::{self, MonitorHandle, VideoMode},
     util::{self, IdRef},
@@ -996,8 +996,6 @@ impl UnownedWindow {
     shared_state_lock.fullscreen = fullscreen.clone();
     trace!("Unlocked shared state in `set_fullscreen`");
 
-    INTERRUPT_EVENT_LOOP_EXIT.store(true, Ordering::SeqCst);
-
     match (&old_fullscreen, &fullscreen) {
       (&None, &Some(_)) => unsafe {
         util::toggle_full_screen_async(
@@ -1067,7 +1065,7 @@ impl UnownedWindow {
           setLevel: ffi::NSWindowLevel::NSNormalWindowLevel
         ];
       },
-      _ => INTERRUPT_EVENT_LOOP_EXIT.store(false, Ordering::SeqCst),
+      _ => {}
     }
     trace!("Unlocked shared state in `set_fullscreen`");
   }

diff --git a/src/platform_impl/macos/window_delegate.rs b/src/platform_impl/macos/window_delegate.rs
@@ -4,7 +4,7 @@
 use std::{
   f64,
   os::raw::c_void,
-  sync::{atomic::Ordering, Arc, Weak},
+  sync::{Arc, Weak},
 };
 
 use cocoa::{
@@ -22,7 +22,7 @@ use crate::{
   event::{Event, WindowEvent},
   keyboard::ModifiersState,
   platform_impl::platform::{
-    app_state::{AppState, INTERRUPT_EVENT_LOOP_EXIT},
+    app_state::AppState,
     event::{EventProxy, EventWrapper},
     util::{self, IdRef},
     view::ViewState,
@@ -480,8 +480,6 @@ extern "C" fn dragging_exited(this: &Object, _: Sel, _: id) {
 extern "C" fn window_will_enter_fullscreen(this: &Object, _: Sel, _: id) {
   trace!("Triggered `windowWillEnterFullscreen:`");
 
-  INTERRUPT_EVENT_LOOP_EXIT.store(true, Ordering::SeqCst);
-
   with_state(this, |state| {
     state.with_window(|window| {
       trace!("Locked shared state in `window_will_enter_fullscreen`");
@@ -514,8 +512,6 @@ extern "C" fn window_will_enter_fullscreen(this: &Object, _: Sel, _: id) {
 extern "C" fn window_will_exit_fullscreen(this: &Object, _: Sel, _: id) {
   trace!("Triggered `windowWillExitFullScreen:`");
 
-  INTERRUPT_EVENT_LOOP_EXIT.store(true, Ordering::SeqCst);
-
   with_state(this, |state| {
     state.with_window(|window| {
       trace!("Locked shared state in `window_will_exit_fullscreen`");
@@ -561,8 +557,6 @@ extern "C" fn window_will_use_fullscreen_presentation_options(
 
 /// Invoked when entered fullscreen
 extern "C" fn window_did_enter_fullscreen(this: &Object, _: Sel, _: id) {
-  INTERRUPT_EVENT_LOOP_EXIT.store(false, Ordering::SeqCst);
-
   trace!("Triggered `windowDidEnterFullscreen:`");
   with_state(this, |state| {
     state.initial_fullscreen = false;
@@ -583,8 +577,6 @@ extern "C" fn window_did_enter_fullscreen(this: &Object, _: Sel, _: id) {
 
 /// Invoked when exited fullscreen
 extern "C" fn window_did_exit_fullscreen(this: &Object, _: Sel, _: id) {
-  INTERRUPT_EVENT_LOOP_EXIT.store(false, Ordering::SeqCst);
-
   trace!("Triggered `windowDidExitFullscreen:`");
   with_state(this, |state| {
     state.with_window(|window| {