Improve macOS menu and tray cleanup logic

- Fix modifier mapping for keyboard accelerators - Add exception safety
to registry cleanup - Ensure delegates and UI references are cleared
before destruction - Prevent callbacks during shutdown and avoid
circular references

Author: lijy91

src/platform/macos/menu_macos.mm

@@ -71,7 +71,7 @@ @interface MenuDelegate : NSObject <NSMenuDelegate>
 
     // Convert modifiers
     if (accelerator.modifiers & KeyboardAccelerator::Ctrl) {
-        modifierMask |= NSEventModifierFlagCommand; // On macOS, Ctrl maps to Cmd
+        modifierMask |= NSEventModifierFlagControl;
     }
     if (accelerator.modifiers & KeyboardAccelerator::Alt) {
         modifierMask |= NSEventModifierFlagOption;
@@ -80,7 +80,7 @@ @interface MenuDelegate : NSObject <NSMenuDelegate>
         modifierMask |= NSEventModifierFlagShift;
     }
     if (accelerator.modifiers & KeyboardAccelerator::Meta) {
-        modifierMask |= NSEventModifierFlagControl; // On macOS, Meta maps to Ctrl
+        modifierMask |= NSEventModifierFlagCommand;
     }
 
     return std::make_pair(keyEquivalent, modifierMask);
@@ -168,7 +168,7 @@ - (void)menuDidClose:(NSMenu *)menu {
         if (submenu_) {
             submenu_.reset();
         }
-        
+
         if (target_) {
             // Remove target and action to prevent callbacks after destruction
             [ns_menu_item_ setTarget:nil];
@@ -227,9 +227,13 @@ - (void)menuDidClose:(NSMenu *)menu {
 
 MenuItem::~MenuItem() {
     // Safely remove from global registry
-    auto it = g_menu_item_registry.find(id);
-    if (it != g_menu_item_registry.end()) {
-        g_menu_item_registry.erase(it);
+    try {
+        auto it = g_menu_item_registry.find(id);
+        if (it != g_menu_item_registry.end()) {
+            g_menu_item_registry.erase(it);
+        }
+    } catch (...) {
+        // Ignore exceptions during cleanup - the registry may be destroyed during shutdown
     }
 }
 
@@ -453,14 +457,14 @@ - (void)menuDidClose:(NSMenu *)menu {
     }
 
     ~Impl() {
-        // First, clear all menu item references
-        items_.clear();
-        
+        // First, remove delegate to prevent callbacks during cleanup
         if (delegate_) {
-            // Remove delegate to prevent callbacks after destruction
             [ns_menu_ setDelegate:nil];
             delegate_ = nil;
         }
+
+        // Then clear all menu item references
+        items_.clear();
     }
 };
 
@@ -492,9 +496,13 @@ - (void)menuDidClose:(NSMenu *)menu {
 
 Menu::~Menu() {
     // Safely remove from global registry
-    auto it = g_menu_registry.find(id);
-    if (it != g_menu_registry.end()) {
-        g_menu_registry.erase(it);
+    try {
+        auto it = g_menu_registry.find(id);
+        if (it != g_menu_registry.end()) {
+            g_menu_registry.erase(it);
+        }
+    } catch (...) {
+        // Ignore exceptions during cleanup - the registry may be destroyed during shutdown
     }
 }
 

src/platform/macos/tray_icon_macos.mm

@@ -39,22 +39,28 @@ - (void)statusItemRightClicked:(id)sender;
   }
 
   ~Impl() {
-    // First, safely clean up context_menu_
-    if (context_menu_) {
-      context_menu_.reset();  // Explicitly reset shared_ptr
+    // First, clean up delegate to prevent callbacks
+    if (delegate_) {
+      delegate_.trayIcon = nullptr;  // Clear the raw pointer first
+      delegate_ = nil;
     }
 
+    // Then clean up the status item
     if (ns_status_item_) {
       // Remove target and action to prevent callbacks after destruction
       if (ns_status_item_.button) {
         [ns_status_item_.button setTarget:nil];
         [ns_status_item_.button setAction:nil];
       }
+      // Clear the menu to break potential circular references
+      [ns_status_item_ setMenu:nil];
       [[NSStatusBar systemStatusBar] removeStatusItem:ns_status_item_];
       ns_status_item_ = nil;
     }
-    if (delegate_) {
-      delegate_ = nil;
+    
+    // Finally, safely clean up context_menu_ after all UI references are cleared
+    if (context_menu_) {
+      context_menu_.reset();  // Explicitly reset shared_ptr
     }
   }
 
@@ -81,6 +87,10 @@ - (void)statusItemRightClicked:(id)sender;
 }
 
 TrayIcon::~TrayIcon() {
+  // Clear the delegate's reference to this object before destruction
+  if (pimpl_ && pimpl_->delegate_) {
+    pimpl_->delegate_.trayIcon = nullptr;
+  }
   delete pimpl_;
 }
 
@@ -289,7 +299,12 @@ - (void)statusItemRightClicked:(id)sender;
 @implementation TrayIconDelegate
 
 - (void)statusItemClicked:(id)sender {
+  // Check if trayIcon is still valid before proceeding
   if (!_trayIcon) return;
+  
+  // Create a local reference to prevent race conditions
+  nativeapi::TrayIcon* trayIcon = _trayIcon;
+  if (!trayIcon) return;
 
   NSEvent* event = [NSApp currentEvent];
   if (!event) return;
@@ -298,23 +313,24 @@ - (void)statusItemClicked:(id)sender {
   if (event.type == NSEventTypeRightMouseUp ||
       (event.type == NSEventTypeLeftMouseUp && (event.modifierFlags & NSEventModifierFlagControl))) {
     // Right click or Ctrl+Left click
-    _trayIcon->HandleRightClick();
+    trayIcon->HandleRightClick();
 
-    // Show context menu if available
-    if (_trayIcon->GetContextMenu()) {
+    // Show context menu if available (check again for safety)
+    if (_trayIcon && _trayIcon->GetContextMenu()) {
       _trayIcon->ShowContextMenu();
     }
   } else if (event.type == NSEventTypeLeftMouseUp) {
     // Check for double click
     if (event.clickCount == 2) {
-      _trayIcon->HandleDoubleClick();
+      trayIcon->HandleDoubleClick();
     } else {
-      _trayIcon->HandleLeftClick();
+      trayIcon->HandleLeftClick();
     }
   }
 }
 
 - (void)statusItemRightClicked:(id)sender {
+  // Check if trayIcon is still valid before proceeding
   if (_trayIcon) {
     _trayIcon->HandleRightClick();
   }

src/platform/macos/tray_manager_macos.mm

@@ -23,16 +23,32 @@
 TrayManager::~TrayManager() {
   std::lock_guard<std::mutex> lock(mutex_);
 
-  // First, clean up all tray icon menu references to prevent circular references
+  // First, hide all tray icons to prevent further UI interactions
   for (auto& pair : trays_) {
     auto tray = pair.second;
     if (tray) {
-      // Explicitly clear menu references
-      tray->SetContextMenu(nullptr);
+      try {
+        tray->Hide();
+      } catch (...) {
+        // Ignore exceptions during cleanup
+      }
     }
   }
 
-  // Then clear the container
+  // Then, clean up all tray icon menu references to prevent circular references
+  for (auto& pair : trays_) {
+    auto tray = pair.second;
+    if (tray) {
+      try {
+        // Explicitly clear menu references
+        tray->SetContextMenu(nullptr);
+      } catch (...) {
+        // Ignore exceptions during cleanup
+      }
+    }
+  }
+  
+  // Finally, clear the container
   trays_.clear();
 }