From c8a82ad2236ee1def621b5930bdb136f01dd07e4 Mon Sep 17 00:00:00 2001 From: Amr Bashir Date: Mon, 15 Apr 2024 12:15:37 +0200 Subject: [PATCH] fix(core): fix deadlock when using resources_table in menu/image/tray plugins (#9379) * fix(core): fix deadlock when using resources_table in menu/image/tray plugins closes #9369 * document the resources_table requirement --- .changes/core-jsimage-intoimg.md | 5 ++ .changes/core-menu-resources-deadlock.md | 5 ++ core/tauri/src/image/mod.rs | 11 +++-- core/tauri/src/menu/plugin.rs | 59 ++++++++++++++---------- core/tauri/src/tray/plugin.rs | 4 +- core/tauri/src/window/plugin.rs | 5 +- 6 files changed, 58 insertions(+), 31 deletions(-) create mode 100644 .changes/core-jsimage-intoimg.md create mode 100644 .changes/core-menu-resources-deadlock.md diff --git a/.changes/core-jsimage-intoimg.md b/.changes/core-jsimage-intoimg.md new file mode 100644 index 000000000..3f4fae5db --- /dev/null +++ b/.changes/core-jsimage-intoimg.md @@ -0,0 +1,5 @@ +--- +"tauri": "patch:breaking" +--- + +Changed `JsImage::into_img` to take a reference to a `ResourceTable` instead of a `Manager`. diff --git a/.changes/core-menu-resources-deadlock.md b/.changes/core-menu-resources-deadlock.md new file mode 100644 index 000000000..5d9924747 --- /dev/null +++ b/.changes/core-menu-resources-deadlock.md @@ -0,0 +1,5 @@ +--- +"tauri": "patch:bug" +--- + +Fix deadlock when using the menu/tray/image JS APIs. diff --git a/core/tauri/src/image/mod.rs b/core/tauri/src/image/mod.rs index e7a3c9c85..87e92aae7 100644 --- a/core/tauri/src/image/mod.rs +++ b/core/tauri/src/image/mod.rs @@ -9,7 +9,7 @@ pub(crate) mod plugin; use std::borrow::Cow; use std::sync::Arc; -use crate::{Manager, Resource, ResourceId, Runtime}; +use crate::{Resource, ResourceId, ResourceTable}; /// An RGBA Image in row-major order from top to bottom. #[derive(Debug, Clone)] @@ -165,9 +165,14 @@ pub enum JsImage { impl JsImage { /// Converts this intermediate image format into an actual [`Image`]. - pub fn into_img>(self, manager: &M) -> crate::Result>> { + /// + /// This will retrieve the image from the passed [`ResourceTable`] if it is [`JsImage::Resource`] + /// and will return an error if it doesn't exist in the passed [`ResourceTable`] so make sure + /// the passed [`ResourceTable`] is the same one used to store the image, usually this should be + /// the webview [resources table](crate::webview::Webview::resources_table). + pub fn into_img(self, resources_table: &ResourceTable) -> crate::Result>> { match self { - Self::Resource(rid) => manager.resources_table().get::>(rid), + Self::Resource(rid) => resources_table.get::>(rid), #[cfg(any(feature = "image-ico", feature = "image-png"))] Self::Path(path) => Image::from_path(path).map(Arc::new).map_err(Into::into), diff --git a/core/tauri/src/menu/plugin.rs b/core/tauri/src/menu/plugin.rs index 2d0a69ed2..fa969bb50 100644 --- a/core/tauri/src/menu/plugin.rs +++ b/core/tauri/src/menu/plugin.rs @@ -15,7 +15,7 @@ use crate::{ plugin::{Builder, TauriPlugin}, resources::ResourceId, sealed::ManagerBase, - Manager, RunEvent, Runtime, State, Webview, Window, + Manager, ResourceTable, RunEvent, Runtime, State, Webview, Window, }; use tauri_macros::do_menu_item; @@ -46,12 +46,12 @@ pub(crate) struct AboutMetadata { } impl AboutMetadata { - pub fn into_metadata>( + pub fn into_metadata( self, - manager: &M, + resources_table: &ResourceTable, ) -> crate::Result> { let icon = match self.icon { - Some(i) => Some(i.into_img(manager)?.as_ref().clone()), + Some(i) => Some(i.into_img(resources_table)?.as_ref().clone()), None => None, }; @@ -102,7 +102,11 @@ struct SubmenuPayload { } impl SubmenuPayload { - pub fn create_item(self, webview: &Webview) -> crate::Result> { + pub fn create_item( + self, + webview: &Webview, + resources_table: &ResourceTable, + ) -> crate::Result> { let mut builder = if let Some(id) = self.id { SubmenuBuilder::with_id(webview, id, self.text) } else { @@ -112,7 +116,7 @@ impl SubmenuPayload { builder = builder.enabled(enabled); } for item in self.items { - builder = item.with_item(webview, |i| Ok(builder.item(i)))?; + builder = item.with_item(webview, resources_table, |i| Ok(builder.item(i)))?; } builder.build() @@ -178,7 +182,11 @@ struct IconMenuItemPayload { } impl IconMenuItemPayload { - pub fn create_item(self, webview: &Webview) -> crate::Result> { + pub fn create_item( + self, + webview: &Webview, + resources_table: &ResourceTable, + ) -> crate::Result> { let mut builder = if let Some(id) = self.id { IconMenuItemBuilder::with_id(id, self.text) } else { @@ -192,7 +200,7 @@ impl IconMenuItemPayload { } builder = match self.icon { Icon::Native(native_icon) => builder.native_icon(native_icon), - Icon::Icon(icon) => builder.icon(icon.into_img(webview)?.as_ref().clone()), + Icon::Icon(icon) => builder.icon(icon.into_img(resources_table)?.as_ref().clone()), }; let item = builder.build(webview)?; @@ -260,6 +268,7 @@ impl PredefinedMenuItemPayload { pub fn create_item( self, webview: &Webview, + resources_table: &ResourceTable, ) -> crate::Result> { match self.item { Predefined::Separator => PredefinedMenuItem::separator(webview), @@ -279,7 +288,7 @@ impl PredefinedMenuItemPayload { Predefined::Quit => PredefinedMenuItem::quit(webview, self.text.as_deref()), Predefined::About(metadata) => { let metadata = match metadata { - Some(m) => Some(m.into_metadata(webview)?), + Some(m) => Some(m.into_metadata(resources_table)?), None => None, }; PredefinedMenuItem::about(webview, self.text.as_deref(), metadata) @@ -304,17 +313,17 @@ impl MenuItemPayloadKind { pub fn with_item) -> crate::Result>( self, webview: &Webview, + resources_table: &ResourceTable, f: F, ) -> crate::Result { match self { Self::ExistingItem((rid, kind)) => { - let resources_table = webview.resources_table(); do_menu_item!(resources_table, rid, kind, |i| f(&*i)) } - Self::Submenu(i) => f(&i.create_item(webview)?), - Self::Predefined(i) => f(&i.create_item(webview)?), + Self::Submenu(i) => f(&i.create_item(webview, resources_table)?), + Self::Predefined(i) => f(&i.create_item(webview, resources_table)?), Self::Check(i) => f(&i.create_item(webview)?), - Self::Icon(i) => f(&i.create_item(webview)?), + Self::Icon(i) => f(&i.create_item(webview, resources_table)?), Self::MenuItem(i) => f(&i.create_item(webview)?), } } @@ -354,7 +363,7 @@ fn new( } if let Some(items) = options.items { for item in items { - builder = item.with_item(&webview, |i| Ok(builder.item(i)))?; + builder = item.with_item(&webview, &resources_table, |i| Ok(builder.item(i)))?; } } let menu = builder.build()?; @@ -371,7 +380,7 @@ fn new( enabled: options.enabled, items: options.items.unwrap_or_default(), } - .create_item(&webview)?; + .create_item(&webview, &resources_table)?; let id = submenu.id().clone(); let rid = resources_table.add(submenu); @@ -398,7 +407,7 @@ fn new( item: options.predefined_item.unwrap(), text: options.text, } - .create_item(&webview)?; + .create_item(&webview, &resources_table)?; let id = item.id().clone(); let rid = resources_table.add(item); (rid, id) @@ -430,7 +439,7 @@ fn new( enabled: options.enabled, accelerator: options.accelerator, } - .create_item(&webview)?; + .create_item(&webview, &resources_table)?; let id = item.id().clone(); let rid = resources_table.add(item); (rid, id) @@ -454,13 +463,13 @@ fn append( ItemKind::Menu => { let menu = resources_table.get::>(rid)?; for item in items { - item.with_item(&webview, |i| menu.append(i))?; + item.with_item(&webview, &resources_table, |i| menu.append(i))?; } } ItemKind::Submenu => { let submenu = resources_table.get::>(rid)?; for item in items { - item.with_item(&webview, |i| submenu.append(i))?; + item.with_item(&webview, &resources_table, |i| submenu.append(i))?; } } _ => return Err(anyhow::anyhow!("unexpected menu item kind").into()), @@ -481,13 +490,13 @@ fn prepend( ItemKind::Menu => { let menu = resources_table.get::>(rid)?; for item in items { - item.with_item(&webview, |i| menu.prepend(i))?; + item.with_item(&webview, &resources_table, |i| menu.prepend(i))?; } } ItemKind::Submenu => { let submenu = resources_table.get::>(rid)?; for item in items { - item.with_item(&webview, |i| submenu.prepend(i))?; + item.with_item(&webview, &resources_table, |i| submenu.prepend(i))?; } } _ => return Err(anyhow::anyhow!("unexpected menu item kind").into()), @@ -509,14 +518,14 @@ fn insert( ItemKind::Menu => { let menu = resources_table.get::>(rid)?; for item in items { - item.with_item(&webview, |i| menu.insert(i, position))?; + item.with_item(&webview, &resources_table, |i| menu.insert(i, position))?; position += 1 } } ItemKind::Submenu => { let submenu = resources_table.get::>(rid)?; for item in items { - item.with_item(&webview, |i| submenu.insert(i, position))?; + item.with_item(&webview, &resources_table, |i| submenu.insert(i, position))?; position += 1 } } @@ -846,7 +855,9 @@ fn set_icon( match icon { Some(Icon::Native(icon)) => icon_item.set_native_icon(Some(icon)), - Some(Icon::Icon(icon)) => icon_item.set_icon(Some(icon.into_img(&webview)?.as_ref().clone())), + Some(Icon::Icon(icon)) => { + icon_item.set_icon(Some(icon.into_img(&resources_table)?.as_ref().clone())) + } None => { icon_item.set_icon(None)?; icon_item.set_native_icon(None)?; diff --git a/core/tauri/src/tray/plugin.rs b/core/tauri/src/tray/plugin.rs index 8b6c09cae..ad335e3f5 100644 --- a/core/tauri/src/tray/plugin.rs +++ b/core/tauri/src/tray/plugin.rs @@ -64,7 +64,7 @@ fn new( }; } if let Some(icon) = options.icon { - builder = builder.icon(icon.into_img(&webview)?.as_ref().clone()); + builder = builder.icon(icon.into_img(&resources_table)?.as_ref().clone()); } if let Some(tooltip) = options.tooltip { builder = builder.tooltip(tooltip); @@ -121,7 +121,7 @@ fn set_icon( let resources_table = webview.resources_table(); let tray = resources_table.get::>(rid)?; let icon = match icon { - Some(i) => Some(i.into_img(&webview)?.as_ref().clone()), + Some(i) => Some(i.into_img(&resources_table)?.as_ref().clone()), None => None, }; tray.set_icon(icon) diff --git a/core/tauri/src/window/plugin.rs b/core/tauri/src/window/plugin.rs index 639dee9a2..035517ff0 100644 --- a/core/tauri/src/window/plugin.rs +++ b/core/tauri/src/window/plugin.rs @@ -19,7 +19,7 @@ mod desktop_commands { sealed::ManagerBase, utils::config::{WindowConfig, WindowEffectsConfig}, window::{ProgressBarState, WindowBuilder}, - AppHandle, CursorIcon, Monitor, PhysicalPosition, PhysicalSize, Position, Size, Theme, + AppHandle, CursorIcon, Manager, Monitor, PhysicalPosition, PhysicalSize, Position, Size, Theme, UserAttentionType, Webview, Window, }; @@ -138,8 +138,9 @@ mod desktop_commands { value: crate::image::JsImage, ) -> crate::Result<()> { let window = get_window(window, label)?; + let resources_table = webview.resources_table(); window - .set_icon(value.into_img(&webview)?.as_ref().clone()) + .set_icon(value.into_img(&resources_table)?.as_ref().clone()) .map_err(Into::into) }