Skip to content

Commit

Permalink
fix(core): fix deadlock when using resources_table in menu/image/tray…
Browse files Browse the repository at this point in the history
… plugins (#9379)

* fix(core): fix deadlock when using resources_table in menu/image/tray plugins

closes #9369

* document the resources_table requirement
  • Loading branch information
amrbashir committed Apr 15, 2024
1 parent 48a7a78 commit c8a82ad
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 31 deletions.
5 changes: 5 additions & 0 deletions .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`.
5 changes: 5 additions & 0 deletions .changes/core-menu-resources-deadlock.md
@@ -0,0 +1,5 @@
---
"tauri": "patch:bug"
---

Fix deadlock when using the menu/tray/image JS APIs.
11 changes: 8 additions & 3 deletions core/tauri/src/image/mod.rs
Expand Up @@ -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)]
Expand Down Expand Up @@ -165,9 +165,14 @@ pub enum JsImage {

impl JsImage {
/// Converts this intermediate image format into an actual [`Image`].
pub fn into_img<R: Runtime, M: Manager<R>>(self, manager: &M) -> crate::Result<Arc<Image<'_>>> {
///
/// 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<Arc<Image<'_>>> {
match self {
Self::Resource(rid) => manager.resources_table().get::<Image<'static>>(rid),
Self::Resource(rid) => resources_table.get::<Image<'static>>(rid),
#[cfg(any(feature = "image-ico", feature = "image-png"))]
Self::Path(path) => Image::from_path(path).map(Arc::new).map_err(Into::into),

Expand Down
59 changes: 35 additions & 24 deletions core/tauri/src/menu/plugin.rs
Expand Up @@ -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;

Expand Down Expand Up @@ -46,12 +46,12 @@ pub(crate) struct AboutMetadata {
}

impl AboutMetadata {
pub fn into_metadata<R: Runtime, M: Manager<R>>(
pub fn into_metadata(
self,
manager: &M,
resources_table: &ResourceTable,
) -> crate::Result<super::AboutMetadata<'_>> {
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,
};

Expand Down Expand Up @@ -102,7 +102,11 @@ struct SubmenuPayload {
}

impl SubmenuPayload {
pub fn create_item<R: Runtime>(self, webview: &Webview<R>) -> crate::Result<Submenu<R>> {
pub fn create_item<R: Runtime>(
self,
webview: &Webview<R>,
resources_table: &ResourceTable,
) -> crate::Result<Submenu<R>> {
let mut builder = if let Some(id) = self.id {
SubmenuBuilder::with_id(webview, id, self.text)
} else {
Expand All @@ -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()
Expand Down Expand Up @@ -178,7 +182,11 @@ struct IconMenuItemPayload {
}

impl IconMenuItemPayload {
pub fn create_item<R: Runtime>(self, webview: &Webview<R>) -> crate::Result<IconMenuItem<R>> {
pub fn create_item<R: Runtime>(
self,
webview: &Webview<R>,
resources_table: &ResourceTable,
) -> crate::Result<IconMenuItem<R>> {
let mut builder = if let Some(id) = self.id {
IconMenuItemBuilder::with_id(id, self.text)
} else {
Expand All @@ -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)?;
Expand Down Expand Up @@ -260,6 +268,7 @@ impl PredefinedMenuItemPayload {
pub fn create_item<R: Runtime>(
self,
webview: &Webview<R>,
resources_table: &ResourceTable,
) -> crate::Result<PredefinedMenuItem<R>> {
match self.item {
Predefined::Separator => PredefinedMenuItem::separator(webview),
Expand All @@ -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)
Expand All @@ -304,17 +313,17 @@ impl MenuItemPayloadKind {
pub fn with_item<T, R: Runtime, F: FnOnce(&dyn IsMenuItem<R>) -> crate::Result<T>>(
self,
webview: &Webview<R>,
resources_table: &ResourceTable,
f: F,
) -> crate::Result<T> {
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)?),
}
}
Expand Down Expand Up @@ -354,7 +363,7 @@ fn new<R: Runtime>(
}
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()?;
Expand All @@ -371,7 +380,7 @@ fn new<R: Runtime>(
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);

Expand All @@ -398,7 +407,7 @@ fn new<R: Runtime>(
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)
Expand Down Expand Up @@ -430,7 +439,7 @@ fn new<R: Runtime>(
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)
Expand All @@ -454,13 +463,13 @@ fn append<R: Runtime>(
ItemKind::Menu => {
let menu = resources_table.get::<Menu<R>>(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::<Submenu<R>>(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()),
Expand All @@ -481,13 +490,13 @@ fn prepend<R: Runtime>(
ItemKind::Menu => {
let menu = resources_table.get::<Menu<R>>(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::<Submenu<R>>(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()),
Expand All @@ -509,14 +518,14 @@ fn insert<R: Runtime>(
ItemKind::Menu => {
let menu = resources_table.get::<Menu<R>>(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::<Submenu<R>>(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
}
}
Expand Down Expand Up @@ -846,7 +855,9 @@ fn set_icon<R: Runtime>(

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)?;
Expand Down
4 changes: 2 additions & 2 deletions core/tauri/src/tray/plugin.rs
Expand Up @@ -64,7 +64,7 @@ fn new<R: Runtime>(
};
}
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);
Expand Down Expand Up @@ -121,7 +121,7 @@ fn set_icon<R: Runtime>(
let resources_table = webview.resources_table();
let tray = resources_table.get::<TrayIcon<R>>(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)
Expand Down
5 changes: 3 additions & 2 deletions core/tauri/src/window/plugin.rs
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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)
}

Expand Down

0 comments on commit c8a82ad

Please sign in to comment.