Browse Source

fix(core): deadlock on window getters, fixes #1893 (#1998)

* fix(core): deadlock on window getters, fixes #1893

* fix compilation without menu feature
Lucas Fernandes Nogueira 4 years ago
parent
commit
ab3eb44bac

+ 6 - 0
.changes/fix-window-getter-deadlock.md

@@ -0,0 +1,6 @@
+---
+"tauri-runtime-wry": patch
+"tauri": patch
+---
+
+Panic on window getters usage on the main thread when the event loop is not running and document it.

+ 22 - 3
core/tauri-runtime-wry/src/lib.rs

@@ -52,14 +52,13 @@ use std::{
   convert::TryFrom,
   fs::read,
   sync::{
+    atomic::{AtomicBool, Ordering},
     mpsc::{channel, Sender},
     Arc, Mutex, MutexGuard,
   },
+  thread::{current as current_thread, ThreadId},
 };
 
-#[cfg(feature = "menu")]
-use std::sync::atomic::{AtomicBool, Ordering};
-
 #[cfg(any(feature = "menu", feature = "system-tray"))]
 mod menu;
 #[cfg(any(feature = "menu", feature = "system-tray"))]
@@ -523,6 +522,8 @@ pub(crate) enum Message {
 
 #[derive(Clone)]
 struct DispatcherContext {
+  main_thread_id: ThreadId,
+  is_event_loop_running: Arc<AtomicBool>,
   proxy: EventLoopProxy<Message>,
   window_event_listeners: WindowEventListeners,
   #[cfg(feature = "menu")]
@@ -538,6 +539,11 @@ pub struct WryDispatcher {
 
 macro_rules! dispatcher_getter {
   ($self: ident, $message: expr) => {{
+    if current_thread().id() == $self.context.main_thread_id
+      && !$self.context.is_event_loop_running.load(Ordering::Relaxed)
+    {
+      panic!("This API cannot be called when the event loop is not running");
+    }
     let (tx, rx) = channel();
     $self
       .context
@@ -954,6 +960,8 @@ struct WebviewWrapper {
 
 /// A Tauri [`Runtime`] wrapper around wry.
 pub struct Wry {
+  main_thread_id: ThreadId,
+  is_event_loop_running: Arc<AtomicBool>,
   event_loop: EventLoop<Message>,
   webviews: Arc<Mutex<HashMap<WindowId, WebviewWrapper>>>,
   window_event_listeners: WindowEventListeners,
@@ -1018,6 +1026,8 @@ impl Runtime for Wry {
   fn new() -> Result<Self> {
     let event_loop = EventLoop::<Message>::with_user_event();
     Ok(Self {
+      main_thread_id: current_thread().id(),
+      is_event_loop_running: Default::default(),
       event_loop,
       webviews: Default::default(),
       window_event_listeners: Default::default(),
@@ -1031,6 +1041,8 @@ impl Runtime for Wry {
   fn handle(&self) -> Self::Handle {
     WryHandle {
       dispatcher_context: DispatcherContext {
+        main_thread_id: self.main_thread_id,
+        is_event_loop_running: self.is_event_loop_running.clone(),
         proxy: self.event_loop.create_proxy(),
         window_event_listeners: self.window_event_listeners.clone(),
         #[cfg(feature = "menu")]
@@ -1048,6 +1060,8 @@ impl Runtime for Wry {
     let webview = create_webview(
       &self.event_loop,
       DispatcherContext {
+        main_thread_id: self.main_thread_id,
+        is_event_loop_running: self.is_event_loop_running.clone(),
         proxy: proxy.clone(),
         window_event_listeners: self.window_event_listeners.clone(),
         #[cfg(feature = "menu")]
@@ -1059,6 +1073,8 @@ impl Runtime for Wry {
     let dispatcher = WryDispatcher {
       window_id: webview.inner.window().id(),
       context: DispatcherContext {
+        main_thread_id: self.main_thread_id,
+        is_event_loop_running: self.is_event_loop_running.clone(),
         proxy,
         window_event_listeners: self.window_event_listeners.clone(),
         #[cfg(feature = "menu")]
@@ -1125,6 +1141,7 @@ impl Runtime for Wry {
 
     let mut iteration = RunIteration::default();
 
+    self.is_event_loop_running.store(true, Ordering::Relaxed);
     self
       .event_loop
       .run_return(|event, event_loop, control_flow| {
@@ -1146,11 +1163,13 @@ impl Runtime for Wry {
           },
         );
       });
+    self.is_event_loop_running.store(false, Ordering::Relaxed);
 
     iteration
   }
 
   fn run<F: Fn() + 'static>(self, callback: F) {
+    self.is_event_loop_running.store(true, Ordering::Relaxed);
     let webviews = self.webviews.clone();
     let window_event_listeners = self.window_event_listeners.clone();
     #[cfg(feature = "menu")]

+ 82 - 0
core/tauri/src/window.rs

@@ -308,16 +308,31 @@ impl<P: Params> Window<P> {
   }
 
   /// Returns the scale factor that can be used to map logical pixels to physical pixels, and vice versa.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn scale_factor(&self) -> crate::Result<f64> {
     self.window.dispatcher.scale_factor().map_err(Into::into)
   }
 
   /// Returns the position of the top-left hand corner of the window's client area relative to the top-left hand corner of the desktop.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn inner_position(&self) -> crate::Result<PhysicalPosition<i32>> {
     self.window.dispatcher.inner_position().map_err(Into::into)
   }
 
   /// Returns the position of the top-left hand corner of the window relative to the top-left hand corner of the desktop.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn outer_position(&self) -> crate::Result<PhysicalPosition<i32>> {
     self.window.dispatcher.outer_position().map_err(Into::into)
   }
@@ -325,6 +340,11 @@ impl<P: Params> Window<P> {
   /// Returns the physical size of the window's client area.
   ///
   /// The client area is the content of the window, excluding the title bar and borders.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn inner_size(&self) -> crate::Result<PhysicalSize<u32>> {
     self.window.dispatcher.inner_size().map_err(Into::into)
   }
@@ -332,31 +352,61 @@ impl<P: Params> Window<P> {
   /// Returns the physical size of the entire window.
   ///
   /// These dimensions include the title bar and borders. If you don't want that (and you usually don't), use inner_size instead.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn outer_size(&self) -> crate::Result<PhysicalSize<u32>> {
     self.window.dispatcher.outer_size().map_err(Into::into)
   }
 
   /// Gets the window's current fullscreen state.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn is_fullscreen(&self) -> crate::Result<bool> {
     self.window.dispatcher.is_fullscreen().map_err(Into::into)
   }
 
   /// Gets the window's current maximized state.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn is_maximized(&self) -> crate::Result<bool> {
     self.window.dispatcher.is_maximized().map_err(Into::into)
   }
 
   /// Gets the window’s current decoration state.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn is_decorated(&self) -> crate::Result<bool> {
     self.window.dispatcher.is_decorated().map_err(Into::into)
   }
 
   /// Gets the window’s current resizable state.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn is_resizable(&self) -> crate::Result<bool> {
     self.window.dispatcher.is_resizable().map_err(Into::into)
   }
 
   /// Gets the window's current vibility state.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn is_visible(&self) -> crate::Result<bool> {
     self.window.dispatcher.is_visible().map_err(Into::into)
   }
@@ -364,6 +414,15 @@ impl<P: Params> Window<P> {
   /// Returns the monitor on which the window currently resides.
   ///
   /// Returns None if current monitor can't be detected.
+  ///
+  /// ## Platform-specific
+  ///
+  /// - **Linux:** Unsupported
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn current_monitor(&self) -> crate::Result<Option<Monitor>> {
     self
       .window
@@ -376,6 +435,15 @@ impl<P: Params> Window<P> {
   /// Returns the primary monitor of the system.
   ///
   /// Returns None if it can't identify any monitor as a primary one.
+  ///
+  /// ## Platform-specific
+  ///
+  /// - **Linux:** Unsupported
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn primary_monitor(&self) -> crate::Result<Option<Monitor>> {
     self
       .window
@@ -386,6 +454,15 @@ impl<P: Params> Window<P> {
   }
 
   /// Returns the list of all the monitors available on the system.
+  ///
+  /// ## Platform-specific
+  ///
+  /// - **Linux:** Unsupported
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn available_monitors(&self) -> crate::Result<Vec<Monitor>> {
     self
       .window
@@ -396,6 +473,11 @@ impl<P: Params> Window<P> {
   }
 
   /// Returns the native handle that is used by this window.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   #[cfg(windows)]
   pub fn hwnd(&self) -> crate::Result<*mut std::ffi::c_void> {
     self.window.dispatcher.hwnd().map_err(Into::into)

+ 10 - 0
core/tauri/src/window/menu.rs

@@ -82,11 +82,21 @@ impl<P: Params> MenuHandle<P> {
   }
 
   /// Whether the menu is visible or not.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn is_visible(&self) -> crate::Result<bool> {
     self.dispatcher.is_menu_visible().map_err(Into::into)
   }
 
   /// Toggles the menu visibility.
+  ///
+  /// # Panics
+  ///
+  /// Panics if the app is not running yet, usually when called on the [`setup`](crate::Builder#method.setup) closure.
+  /// You can spawn a task to use the API using the [`async_runtime`](crate::async_runtime) to prevent the panic.
   pub fn toggle(&self) -> crate::Result<()> {
     if self.is_visible()? {
       self.hide()