-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(linux): correct single-instance detection order #427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughRelocated IPC server cleanup from before the duplicate-instance check to after it so cleanup runs only when a new instance proceeds (no existing instance detected), preserving post-exit cleanup of stale IPC artifacts. Changes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@linux/main.cpp`:
- Around line 1026-1028: Remove the hardcoded QFile-based removals of
"/tmp/app_server" and rely solely on QLocalServer::removeServer("app_server");
specifically delete the QFile stale("/tmp/app_server"); if (stale.exists())
stale.remove(); blocks (the occurrences around the QLocalServer::removeServer
calls) so the code no longer attempts to manually remove /tmp/app_server and
lets QLocalServer handle socket cleanup in the correct runtime directory.
🧹 Nitpick comments (1)
linux/main.cpp (1)
1061-1064: RedundantremoveServercall.Line 1062 is now redundant since cleanup already occurs at line 1025 after the connection check fails.
🔧 Suggested fix
QLocalServer server; - QLocalServer::removeServer("app_server"); if (!server.listen("app_server"))
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
linux/main.cpp
🔇 Additional comments (1)
linux/main.cpp (1)
1013-1028: LGTM! Correct fix for single-instance detection.The reordering properly ensures the cleanup only runs after confirming no existing instance is running. This prevents the race condition where a new launch would delete the socket before detecting the already-running instance.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
problem
When launching the app while an instance is already running, a new instance starts instead of bringing the existing window to focus. This results in duplicate system tray icons.
The socket cleanup was running BEFORE the connection check,causing every new launch to delete the socket before attempting to detect an existing instance.
solution
Reorder the logic in
main():Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.