[client] Fix tray menu item leak on relayout - #6939
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughTray menu rebuilding now destroys replaced menus, asynchronous connect/disconnect handlers resolve current menu items safely, session labels update under the menu lock, and profile loading relayouts only when relevant data changes. ChangesTray menu updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/ui/tray.go`:
- Around line 70-71: Update the tray item enablement flow around the visible
menuMu locking logic to keep menuMu held from locating the current item through
completion of item.SetEnabled. Remove the guidance to snapshot under the lock
and call the item afterward, ensuring relayoutMenu cannot rebuild or destroy the
item before SetEnabled returns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c75ca01b-cf35-483c-8280-d50ab3ee4968
📒 Files selected for processing (2)
client/ui/tray.goclient/ui/tray_profiles.go
Release artifactsBuilt for PR head
GHCR images (amd64)
This comment is updated by the Release workflow. Artifact links expire according to the workflow retention policy. |
|
2 participants
Describe your changes
Fix a memory leak in the tray menu: every relayout built a brand-new menu tree while the replaced one was never destroyed. Wails keeps each MenuItem in a process-global map until Destroy is called, so all items of every previous tree
(labels, click closures, bitmaps, submenus) stayed reachable forever and the GUI's memory grew with every status transition, profile refresh or language change.
The fix destroys the replaced tree right after SetMenu swaps in the new one. Since the old items can now be destroyed, the Connect/Disconnect click handlers no longer capture their own menu item for the async error path; they resolve
the current item under the menu lock instead. Additionally loadProfiles now relayouts only when the profile rows actually changed, which drops the redundant full menu rebuilds (previously 2-3 per status transition, now typically 1).
Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit