From 6898aa04bb57715f1b5322ac9d1a4670a18a6088 Mon Sep 17 00:00:00 2001 From: Jason Tsai Date: Thu, 14 Sep 2023 19:11:53 +0800 Subject: [PATCH 1/5] fix(macOS): remove autorelease from ns_submenu to avoid over release --- src/platform_impl/macos/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index 778b8c1a..4e0ebbb7 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -669,7 +669,9 @@ impl MenuChild { unsafe { ns_menu_item = NSMenuItem::alloc(nil).autorelease(); - ns_submenu = NSMenu::alloc(nil).autorelease(); + + // ns_submenu will be manually released when NsMenuRef dropped + ns_submenu = NSMenu::alloc(nil); let title = NSString::alloc(nil).init_str(&self.text).autorelease(); let () = msg_send![ns_submenu, setTitle: title]; From f2dc7dad918c19904430cf3c4463a8248383f236 Mon Sep 17 00:00:00 2001 From: Jason Tsai Date: Thu, 14 Sep 2023 23:37:58 +0800 Subject: [PATCH 2/5] refactor(macOS): implement NsMenuItemRef to release NSMenuItem manually --- src/platform_impl/macos/mod.rs | 57 ++++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index 4e0ebbb7..117d9b8a 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -59,6 +59,17 @@ impl Drop for NsMenuRef { } } +#[derive(Debug, Clone)] +struct NsMenuItemRef(id); + +impl Drop for NsMenuItemRef { + fn drop(&mut self) { + unsafe { + let _: () = msg_send![self.0, releasse]; + } + } +} + #[derive(Debug)] pub struct Menu { id: MenuId, @@ -185,7 +196,7 @@ pub struct MenuChild { text: String, enabled: bool, - ns_menu_items: HashMap>, + ns_menu_items: HashMap>, // menu item fields accelerator: Option, @@ -416,9 +427,9 @@ impl MenuChild { unsafe { let title = NSString::alloc(nil).init_str(&self.text).autorelease(); for ns_items in self.ns_menu_items.values() { - for &ns_item in ns_items { - let () = msg_send![ns_item, setTitle: title]; - let ns_submenu: id = msg_send![ns_item, submenu]; + for ns_item in ns_items { + let () = msg_send![ns_item.0, setTitle: title]; + let ns_submenu: id = msg_send![ns_item.0, submenu]; if ns_submenu != nil { let () = msg_send![ns_submenu, setTitle: title]; } @@ -434,9 +445,9 @@ impl MenuChild { pub fn set_enabled(&mut self, enabled: bool) { self.enabled = enabled; for ns_items in self.ns_menu_items.values() { - for &ns_item in ns_items { + for ns_item in ns_items { unsafe { - let () = msg_send![ns_item, setEnabled: if enabled { YES } else { NO }]; + let () = msg_send![ns_item.0, setEnabled: if enabled { YES } else { NO }]; } } } @@ -461,10 +472,10 @@ impl MenuChild { .unwrap_or_else(NSEventModifierFlags::empty); for ns_items in self.ns_menu_items.values() { - for &ns_item in ns_items { + for ns_item in ns_items { unsafe { - let _: () = msg_send![ns_item, setKeyEquivalent: key_equivalent]; - ns_item.setKeyEquivalentModifierMask_(modifier_mask); + let _: () = msg_send![ns_item.0, setKeyEquivalent: key_equivalent]; + ns_item.0.setKeyEquivalentModifierMask_(modifier_mask); } } } @@ -485,9 +496,9 @@ impl MenuChild { pub fn set_checked(&mut self, checked: bool) { self.checked = checked; for ns_items in self.ns_menu_items.values() { - for &ns_item in ns_items { + for ns_item in ns_items { unsafe { - let () = msg_send![ns_item, setState: checked as u32]; + let () = msg_send![ns_item.0, setState: checked as u32]; } } } @@ -500,8 +511,8 @@ impl MenuChild { self.icon = icon.clone(); self.native_icon = None; for ns_items in self.ns_menu_items.values() { - for &ns_item in ns_items { - menuitem_set_icon(ns_item, icon.as_ref()); + for ns_item in ns_items { + menuitem_set_icon(ns_item.0, icon.as_ref()); } } } @@ -510,8 +521,8 @@ impl MenuChild { self.native_icon = icon; self.icon = None; for ns_items in self.ns_menu_items.values() { - for &ns_item in ns_items { - menuitem_set_native_icon(ns_item, icon); + for ns_item in ns_items { + menuitem_set_native_icon(ns_item.0, icon); } } } @@ -668,9 +679,7 @@ impl MenuChild { let ns_submenu: id; unsafe { - ns_menu_item = NSMenuItem::alloc(nil).autorelease(); - - // ns_submenu will be manually released when NsMenuRef dropped + ns_menu_item = NSMenuItem::alloc(nil); ns_submenu = NSMenu::alloc(nil); let title = NSString::alloc(nil).init_str(&self.text).autorelease(); @@ -700,7 +709,7 @@ impl MenuChild { self.ns_menu_items .entry(menu_id) .or_insert_with(Vec::new) - .push(ns_menu_item); + .push(NsMenuItemRef(ns_menu_item)); Ok(ns_menu_item) } @@ -727,7 +736,7 @@ impl MenuChild { self.ns_menu_items .entry(menu_id) .or_insert_with(Vec::new) - .push(ns_menu_item); + .push(NsMenuItemRef(ns_menu_item)); Ok(ns_menu_item) } @@ -764,7 +773,7 @@ impl MenuChild { self.ns_menu_items .entry(menu_id) .or_insert_with(Vec::new) - .push(ns_menu_item); + .push(NsMenuItemRef(ns_menu_item)); Ok(ns_menu_item) } @@ -794,7 +803,7 @@ impl MenuChild { self.ns_menu_items .entry(menu_id) .or_insert_with(Vec::new) - .push(ns_menu_item); + .push(NsMenuItemRef(ns_menu_item)); Ok(ns_menu_item) } @@ -827,7 +836,7 @@ impl MenuChild { self.ns_menu_items .entry(menu_id) .or_insert_with(Vec::new) - .push(ns_menu_item); + .push(NsMenuItemRef(ns_menu_item)); Ok(ns_menu_item) } @@ -1027,7 +1036,7 @@ fn create_ns_menu_item( ns_menu_item.initWithTitle_action_keyEquivalent_(title, selector, key_equivalent); ns_menu_item.setKeyEquivalentModifierMask_(modifier_mask); - Ok(ns_menu_item.autorelease()) + Ok(ns_menu_item) } } From c00a4a32cc5918895ce1aa656370d6c9b746f640 Mon Sep 17 00:00:00 2001 From: Jason Tsai Date: Thu, 14 Sep 2023 23:41:57 +0800 Subject: [PATCH 3/5] fix: typo --- src/platform_impl/macos/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform_impl/macos/mod.rs b/src/platform_impl/macos/mod.rs index 117d9b8a..44164b1d 100644 --- a/src/platform_impl/macos/mod.rs +++ b/src/platform_impl/macos/mod.rs @@ -65,7 +65,7 @@ struct NsMenuItemRef(id); impl Drop for NsMenuItemRef { fn drop(&mut self) { unsafe { - let _: () = msg_send![self.0, releasse]; + let _: () = msg_send![self.0, release]; } } } From 9719f2e4f8062979f5de70a4f32fd3d26a08b4ca Mon Sep 17 00:00:00 2001 From: Amr Bashir Date: Mon, 18 Sep 2023 17:46:23 +0300 Subject: [PATCH 4/5] chnage file --- .changes/separator-autorelease.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/separator-autorelease.md diff --git a/.changes/separator-autorelease.md b/.changes/separator-autorelease.md new file mode 100644 index 00000000..ccbc4f50 --- /dev/null +++ b/.changes/separator-autorelease.md @@ -0,0 +1,5 @@ +--- +"muda": patch +--- + +On macOS, fix menu crash due to a double free when using `PredefinedMenuItem::separator` \ No newline at end of file From 197be6839ca6effbca30ccedec3d4af10cd5e2a3 Mon Sep 17 00:00:00 2001 From: Amr Bashir Date: Mon, 18 Sep 2023 17:52:49 +0300 Subject: [PATCH 5/5] Update separator-autorelease.md --- .changes/separator-autorelease.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/separator-autorelease.md b/.changes/separator-autorelease.md index ccbc4f50..886c539d 100644 --- a/.changes/separator-autorelease.md +++ b/.changes/separator-autorelease.md @@ -2,4 +2,4 @@ "muda": patch --- -On macOS, fix menu crash due to a double free when using `PredefinedMenuItem::separator` \ No newline at end of file +On macOS, fix menu crash due to a double freeing the underlying NsMenu.