feat: single-instance application support#407
feat: single-instance application support#407tyvsmith wants to merge 2 commits intofeschber:mainfrom
Conversation
Previously, every launch of lan-mouse in GUI mode unconditionally spawned a new daemon child process and created a new GTK window, even if an instance was already running. The daemon had single-instance detection (via IPC socket binding), but the GUI layer did not take advantage of it. This change makes the full application single-instance: - main.rs: Before spawning a daemon child process, probe the IPC socket via is_service_running(). If a daemon is already listening, skip spawning and just launch the GTK frontend, which connects to the existing daemon. Daemon lifecycle cleanup (SIGINT + wait + kill) is only performed if we spawned the daemon ourselves. - lan-mouse-ipc: Add is_service_running() that does a one-shot socket probe (Unix socket or TCP on Windows) to check if a daemon is reachable. Add try_connect() that attempts a single connection without the infinite retry loop of connect()/wait_for_service(). Extract shared connection-building logic into make_connection(). - lan-mouse-gtk: In build_ui(), check app.active_window() first and present the existing window on GApplication re-activation instead of creating a duplicate window with a new IPC connection. Replace the blocking connect() call with try_connect(), falling back to spawning a new daemon and then using connect() if the initial connection fails. This handles the edge case where a daemon dies between the main.rs probe and the GTK connection attempt. Behavior changes to note: - When connecting to an externally-managed daemon (e.g. one started via `lan-mouse daemon`), closing the GUI no longer kills it. - If the daemon dies between startup detection and GTK connection, the GUI transparently spawns a replacement instead of hanging. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Implements single-instance behavior for the GUI by avoiding redundant daemon spawns and reusing/presenting existing frontend windows.
Changes:
src/main.rs: Avoid spawning a daemon when one is already running; only clean up the daemon process if this instance spawned it.lan-mouse-ipc: Addis_service_running()plus a non-retryingtry_connect(), and factor shared connection setup intomake_connection().lan-mouse-gtk: Present an existing window on re-activation; attempttry_connect()first and spawn a daemon only if initial connection fails.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/main.rs |
Adds daemon-running probe before spawning and gates daemon cleanup on whether it was spawned locally. |
lan-mouse-ipc/src/lib.rs |
Exposes try_connect() and introduces is_service_running() to probe IPC availability. |
lan-mouse-ipc/src/connect.rs |
Implements try_connect() and refactors connection creation into make_connection(). |
lan-mouse-gtk/src/lib.rs |
Presents an existing window on activate; adds fallback daemon spawn if initial IPC connection fails. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let Ok(socket_path) = default_socket_path() else { | ||
| return false; | ||
| }; | ||
| std::os::unix::net::UnixStream::connect(socket_path).is_ok() |
There was a problem hiding this comment.
is_service_running() establishes a real IPC connection to probe liveness. In AsyncFrontendListener::poll_next any accepted connection triggers a FrontendRequest::Sync, so this probe will cause the daemon to broadcast a full sync to existing frontends every time it’s called (even though the probe immediately disconnects). Consider using a side-effect-free single-instance mechanism (e.g., lockfile/named mutex) or changing the listener/protocol so that a liveness probe doesn’t register as a frontend connection that triggers Sync.
| std::os::unix::net::UnixStream::connect(socket_path).is_ok() | |
| // Avoid establishing a real IPC connection, which would be interpreted as a | |
| // frontend connection by the daemon and trigger a sync broadcast. Checking | |
| // for the existence of the socket path is a side-effect-free approximation. | |
| std::fs::metadata(socket_path).is_ok() |
There was a problem hiding this comment.
The observation about the spurious Sync is valid, but the suggested fix (std::fs::metadata) would break single-instance detection — a stale socket file left behind by a crashed daemon would give a false positive, and we'd skip spawning when no daemon is actually running. The existing listener code in listen.rs already handles this case correctly by attempting a real connection to distinguish stale sockets from live daemons.
The extra Sync is harmless — it's one redundant state broadcast at startup. Eliminating the double-connect would require passing the connection across the crate boundary (main.rs → GTK), which is a larger refactor for marginal gain.
lan-mouse-gtk/src/lib.rs
Outdated
|
|
||
| fn build_ui(app: &Application) { | ||
| // If a window already exists (re-activation), just present it | ||
| if let Some(window) = app.active_window() { |
There was a problem hiding this comment.
build_ui checks app.active_window() to avoid creating a second window, but active_window() can be None even when a window already exists (e.g., the existing window isn’t currently active/focused). This can still create duplicates. Prefer checking whether any window exists (e.g., via the application’s window list) and presenting that instead.
| if let Some(window) = app.active_window() { | |
| if let Some(window) = app.windows().first() { |
There was a problem hiding this comment.
Good catch — active_window() only returns the focused window, so a window that exists but isn't focused would be missed. Fixed to use app.windows().first() instead.
| let (mut frontend_rx, frontend_tx) = match lan_mouse_ipc::try_connect() { | ||
| Ok(conn) => conn, | ||
| Err(e) => { | ||
| log::error!("{e}"); | ||
| process::exit(1); | ||
| log::warn!("could not connect to daemon ({e}), spawning a new one"); | ||
| if let Err(spawn_err) = process::Command::new( | ||
| env::current_exe().expect("could not determine executable path"), | ||
| ) | ||
| .args(env::args().skip(1)) | ||
| .arg("daemon") | ||
| .spawn() | ||
| { | ||
| log::error!("failed to spawn daemon: {spawn_err}"); | ||
| process::exit(1); | ||
| } |
There was a problem hiding this comment.
When try_connect() fails, the GTK frontend spawns a daemon process but discards the Child handle, so it cannot be terminated on GUI exit. In the race where main.rs decides not to spawn (because the daemon was running) but the daemon dies before GTK connects, this path will leave a newly spawned daemon running after the GUI closes. Consider centralizing daemon spawning/lifecycle management in main.rs (or returning the child handle/ownership information) so that only daemons spawned by the GUI are cleaned up.
There was a problem hiding this comment.
This is by design. The fallback path only triggers in a rare race condition (daemon dies between main.rs probe and GTK connect). The spawned daemon runs independently, same as if the user had run lan-mouse daemon separately — it manages its own lifecycle and cleans up its socket on drop. Centralizing the lifecycle would require threading a Child handle back through the GTK crate boundary, adding complexity for an edge case where the intended behavior (daemon keeps running) is already correct.
| // Only spawn a new daemon if one isn't already running | ||
| let mut service = if lan_mouse_ipc::is_service_running() { | ||
| log::info!("daemon already running, connecting to existing instance"); | ||
| None | ||
| } else { | ||
| Some(start_service()?) | ||
| }; | ||
| let res = lan_mouse_gtk::run(); |
There was a problem hiding this comment.
This path probes the daemon via is_service_running() and then the GTK frontend attempts try_connect() again, resulting in two connections per launch when a daemon is already running. Besides the extra work, the probe connection can trigger a spurious FrontendRequest::Sync broadcast from the daemon (see listener behavior). Consider relying on a single try_connect() attempt in one place to decide whether to spawn, rather than probing in main.rs and reconnecting in GTK.
There was a problem hiding this comment.
Valid observation — there are two connections: the is_service_running() probe in main.rs and try_connect() in GTK. Consolidating would require either passing the IPC connection from main.rs into the GTK crate (different API boundaries) or removing the main.rs probe and letting GTK handle all spawning (losing the clean lifecycle management in main.rs). The cost is one extra harmless Sync broadcast at startup, which doesn't seem worth the refactor.
| service.wait()?; | ||
| } | ||
| service.wait()?; | ||
| service.kill()?; |
There was a problem hiding this comment.
On Unix, this sends SIGINT and then calls wait(), which reaps the child. Calling service.kill()? afterwards is unnecessary and can fail with an OS error (process already exited/reaped), causing run() to error during normal shutdown. Consider only killing if the process is still running (e.g., via try_wait()), or ignore/handle the expected "already exited" error.
| service.kill()?; | |
| #[cfg(not(unix))] | |
| { | |
| service.kill()?; | |
| } |
There was a problem hiding this comment.
This is pre-existing behavior — the original code had the same wait() then kill() sequence. This PR just moved it inside the if let Some guard. Fixing the redundant kill is a good cleanup but out of scope for this change.
…on check active_window() only returns the focused window, which can be None even when a window exists but isn't focused. Use windows().first() to check if any window exists regardless of focus state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Fixes #406 — makes lan-mouse a single-instance application so that repeated GUI launches reuse the existing daemon and raise the existing window.
is_service_running()before spawning a daemon child process. If a daemon is already listening, skip spawning and connect the GTK frontend to it. Daemon lifecycle cleanup (SIGINT + wait + kill) only runs if we spawned the daemon ourselves.is_service_running()for a one-shot socket probe (Unix socket / TCP on Windows). Addtry_connect()that attempts a single connection without the infinite retry loop ofconnect()/wait_for_service(). Extract shared connection-building intomake_connection().app.active_window()on activate and present the existing window instead of creating a duplicate. Usetry_connect()first, falling back to spawning a new daemon +connect()if the connection fails — handles the edge case where a daemon dies between the main.rs probe and the GTK connection attempt.Behavior changes
lan-mouse daemon), closing the GUI no longer kills it.Test plan
Verified locally on Arch Linux + Hyprland (Wayland):
lan-mouse daemonstarted separately, thenlan-mouseGUI: connects to existing daemon, closing GUI leaves daemon runningcargo fmtpassescargo buildsucceeds🤖 Generated with Claude Code