Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(core): Replace Rc with Arc to prevent crashes when sending events #8402

Merged
merged 5 commits into from
Dec 20, 2023
Merged

fix(core): Replace Rc with Arc to prevent crashes when sending events #8402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
444a2a2
fix(core): Prevent crash when sending events.
FabianLars Dec 12, 2023
dd16255
Merge branch '1.x' into event-crash
lucasfernog Dec 14, 2023
06050da
add change file
lucasfernog Dec 14, 2023
2920718
Merge branch '1.x' into event-crash
FabianLars Dec 19, 2023
bb286ee
use dedicated type for windows refcell map
lucasfernog Dec 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/prevent-crash.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"tauri-runtime-wry": patch:bug
---

Use `Arc` instead of `Rc` to prevent crashes on macOS.
63 changes: 41 additions & 22 deletions core/tauri-runtime-wry/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,13 +268,25 @@ pub enum ActiveTracingSpan {
},
}

#[derive(Debug)]
pub struct WindowsStore(RefCell<HashMap<WebviewId, WindowWrapper>>);

// SAFETY: we ensure this type is only used on the main thread.
#[allow(clippy::non_send_fields_in_send_ty)]
unsafe impl Send for WindowsStore {}

// SAFETY: we ensure this type is only used on the main thread.
#[allow(clippy::non_send_fields_in_send_ty)]
unsafe impl Sync for WindowsStore {}

#[derive(Debug, Clone)]
pub struct DispatcherMainThreadContext<T: UserEvent> {
pub window_target: EventLoopWindowTarget<Message<T>>,
pub web_context: WebContextStore,
#[cfg(all(desktop, feature = "global-shortcut"))]
pub global_shortcut_manager: Rc<Mutex<WryShortcutManager>>,
pub windows: Rc<RefCell<HashMap<WebviewId, WindowWrapper>>>,
// changing this to an Rc will cause frequent app crashes.
pub windows: Arc<WindowsStore>,
#[cfg(all(desktop, feature = "system-tray"))]
system_tray_manager: SystemTrayManager,
#[cfg(feature = "tracing")]
Expand Down Expand Up @@ -1973,7 +1985,7 @@ impl<T: UserEvent> Wry<T> {
#[cfg(all(desktop, feature = "global-shortcut"))]
let global_shortcut_manager = Rc::new(Mutex::new(WryShortcutManager::new(&event_loop)));

let windows = Rc::new(RefCell::new(HashMap::default()));
let windows = Arc::new(WindowsStore(RefCell::new(HashMap::default())));
let webview_id_map = WebviewIdStore::default();

#[cfg(all(desktop, feature = "system-tray"))]
Expand Down Expand Up @@ -2104,6 +2116,7 @@ impl<T: UserEvent> Runtime<T> for Wry<T> {
.context
.main_thread
.windows
.0
.borrow_mut()
.insert(window_id, webview);

Expand Down Expand Up @@ -2337,7 +2350,7 @@ impl<T: UserEvent> Runtime<T> for Wry<T> {
pub struct EventLoopIterationContext<'a, T: UserEvent> {
pub callback: &'a mut (dyn FnMut(RunEvent<T>) + 'static),
pub webview_id_map: WebviewIdStore,
pub windows: Rc<RefCell<HashMap<WebviewId, WindowWrapper>>>,
pub windows: Arc<WindowsStore>,
#[cfg(all(desktop, feature = "global-shortcut"))]
pub global_shortcut_manager: Rc<Mutex<WryShortcutManager>>,
#[cfg(all(desktop, feature = "global-shortcut"))]
Expand All @@ -2349,7 +2362,7 @@ pub struct EventLoopIterationContext<'a, T: UserEvent> {
}

struct UserMessageContext {
windows: Rc<RefCell<HashMap<WebviewId, WindowWrapper>>>,
windows: Arc<WindowsStore>,
webview_id_map: WebviewIdStore,
#[cfg(all(desktop, feature = "global-shortcut"))]
global_shortcut_manager: Rc<Mutex<WryShortcutManager>>,
Expand Down Expand Up @@ -2384,7 +2397,12 @@ fn handle_user_message<T: UserEvent>(
},
Message::Window(id, window_message) => {
if let WindowMessage::UpdateMenuItem(item_id, update) = window_message {
if let Some(menu_items) = windows.borrow_mut().get_mut(&id).map(|w| &mut w.menu_items) {
if let Some(menu_items) = windows
.0
.borrow_mut()
.get_mut(&id)
.map(|w| &mut w.menu_items)
{
if let Some(menu_items) = menu_items.as_mut() {
let item = menu_items.get_mut(&item_id).expect("menu item not found");
match update {
Expand All @@ -2399,7 +2417,7 @@ fn handle_user_message<T: UserEvent>(
}
}
} else {
let w = windows.borrow().get(&id).map(|w| {
let w = windows.0.borrow().get(&id).map(|w| {
(
w.inner.clone(),
w.window_event_listeners.clone(),
Expand Down Expand Up @@ -2622,7 +2640,7 @@ fn handle_user_message<T: UserEvent>(
WebviewMessage::EvaluateScript(script, tx, span) => {
let _span = span.entered();
if let Some(WindowHandle::Webview { inner: webview, .. }) =
windows.borrow().get(&id).and_then(|w| w.inner.as_ref())
windows.0.borrow().get(&id).and_then(|w| w.inner.as_ref())
{
if let Err(e) = webview.evaluate_script(&script) {
debug_eprintln!("{}", e);
Expand All @@ -2633,7 +2651,7 @@ fn handle_user_message<T: UserEvent>(
#[cfg(not(feature = "tracing"))]
WebviewMessage::EvaluateScript(script) => {
if let Some(WindowHandle::Webview { inner: webview, .. }) =
windows.borrow().get(&id).and_then(|w| w.inner.as_ref())
windows.0.borrow().get(&id).and_then(|w| w.inner.as_ref())
{
if let Err(e) = webview.evaluate_script(&script) {
debug_eprintln!("{}", e);
Expand All @@ -2642,7 +2660,7 @@ fn handle_user_message<T: UserEvent>(
}
WebviewMessage::Print => {
if let Some(WindowHandle::Webview { inner: webview, .. }) =
windows.borrow().get(&id).and_then(|w| w.inner.as_ref())
windows.0.borrow().get(&id).and_then(|w| w.inner.as_ref())
{
let _ = webview.print();
}
Expand All @@ -2651,7 +2669,7 @@ fn handle_user_message<T: UserEvent>(
},
Message::CreateWebview(window_id, handler) => match handler(event_loop, web_context) {
Ok(webview) => {
windows.borrow_mut().insert(window_id, webview);
windows.0.borrow_mut().insert(window_id, webview);
}
Err(e) => {
debug_eprintln!("{}", e);
Expand All @@ -2664,7 +2682,7 @@ fn handle_user_message<T: UserEvent>(

let w = Arc::new(window);

windows.borrow_mut().insert(
windows.0.borrow_mut().insert(
window_id,
WindowWrapper {
label,
Expand Down Expand Up @@ -2773,7 +2791,7 @@ fn handle_user_message<T: UserEvent>(
}

let it = RunIteration {
window_count: windows.borrow().len(),
window_count: windows.0.borrow().len(),
};
it
}
Expand Down Expand Up @@ -2861,6 +2879,7 @@ fn handle_event_loop<T: UserEvent>(
*webview_id_map.0.lock().unwrap().values().next().unwrap()
};
windows
.0
.borrow()
.get(&window_id)
.unwrap()
Expand Down Expand Up @@ -2946,7 +2965,7 @@ fn handle_event_loop<T: UserEvent>(
}
Event::UserEvent(Message::Webview(id, WebviewMessage::WebviewEvent(event))) => {
if let Some(event) = WindowEventWrapper::from(&event).0 {
let windows = windows.borrow();
let windows = windows.0.borrow();
let window = windows.get(&id);
if let Some(window) = window {
callback(RunEvent::WindowEvent {
Expand All @@ -2967,7 +2986,7 @@ fn handle_event_loop<T: UserEvent>(
} => {
if let Some(window_id) = webview_id_map.get(&window_id) {
{
let windows_ref = windows.borrow();
let windows_ref = windows.0.borrow();
if let Some(window) = windows_ref.get(&window_id) {
if let Some(event) = WindowEventWrapper::parse(&window.inner, &event).0 {
let label = window.label.clone();
Expand All @@ -2991,7 +3010,7 @@ fn handle_event_loop<T: UserEvent>(
match event {
#[cfg(windows)]
WryWindowEvent::ThemeChanged(theme) => {
if let Some(window) = windows.borrow().get(&window_id) {
if let Some(window) = windows.0.borrow().get(&window_id) {
if let Some(WindowHandle::Webview { inner, .. }) = &window.inner {
let theme = match theme {
WryTheme::Dark => wry::webview::Theme::Dark,
Expand All @@ -3006,9 +3025,9 @@ fn handle_event_loop<T: UserEvent>(
on_close_requested(callback, window_id, windows.clone());
}
WryWindowEvent::Destroyed => {
let removed = windows.borrow_mut().remove(&window_id).is_some();
let removed = windows.0.borrow_mut().remove(&window_id).is_some();
if removed {
let is_empty = windows.borrow().is_empty();
let is_empty = windows.0.borrow().is_empty();
if is_empty {
let (tx, rx) = channel();
callback(RunEvent::ExitRequested { tx });
Expand Down Expand Up @@ -3051,18 +3070,18 @@ fn handle_event_loop<T: UserEvent>(
}

let it = RunIteration {
window_count: windows.borrow().len(),
window_count: windows.0.borrow().len(),
};
it
}

fn on_close_requested<'a, T: UserEvent>(
callback: &'a mut (dyn FnMut(RunEvent<T>) + 'static),
window_id: WebviewId,
windows: Rc<RefCell<HashMap<WebviewId, WindowWrapper>>>,
windows: Arc<WindowsStore>,
) {
let (tx, rx) = channel();
let windows_ref = windows.borrow();
let windows_ref = windows.0.borrow();
if let Some(w) = windows_ref.get(&window_id) {
let label = w.label.clone();
let window_event_listeners = w.window_event_listeners.clone();
Expand All @@ -3087,8 +3106,8 @@ fn on_close_requested<'a, T: UserEvent>(
}
}

fn on_window_close(window_id: WebviewId, windows: Rc<RefCell<HashMap<WebviewId, WindowWrapper>>>) {
if let Some(window_wrapper) = windows.borrow_mut().get_mut(&window_id) {
fn on_window_close(window_id: WebviewId, windows: Arc<WindowsStore>) {
if let Some(window_wrapper) = windows.0.borrow_mut().get_mut(&window_id) {
window_wrapper.inner = None;
}
}
Expand Down
Loading