Skip to content

Terminate duplicate Finicky instances at launch (fixes #515) - #516

Merged
johnste merged 2 commits into
johnste:mainfrom
joeflateau:fix/single-instance-guard
Apr 18, 2026
Merged

johnste merged 2 commits into
johnste:mainfrom
joeflateau:fix/single-instance-guard

Conversation

@joeflateau

@joeflateau joeflateau commented Apr 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes the menu-bar-pile-up described in #515: Finicky has no single-instance guard, so whenever macOS spawns a second process (stale Launch Services registration after app moves/updates, different bundle paths, SSO/enterprise-auth agents launching from another context) a new status bar item accumulates and never gets reaped.

Adds a simple guard at the top of applicationDidFinishLaunching: in apps/finicky/src/main.m: enumerate [NSRunningApplication runningApplicationsWithBundleIdentifier:] and call [app terminate] (falling back to forceTerminate) on any instance that isn't us. Normal operation is unaffected — there's nothing to terminate when no duplicates exist.

Why this approach

  • No LSMultipleInstancesProhibited in Info.plist and no runtime check previously existed; the app relied entirely on Launch Services routing GetURL events to the existing instance, which fails in the scenarios above.
  • Terminating other instances (rather than self-terminating) means the newest launch wins. This is what you want when the existing instance is stale (old bundle path, zombie from a prior user session, etc.), which is exactly the failure mode users report.
  • Simple and bounded — no IPC, no lock files, no restructuring of the launch flow.

Test plan

Verified locally:

  • Built the patched app as Finicky-test.app (via BUILD_TARGET_ARCH=test ./scripts/build.sh).
  • Before launch, pgrep -fl Finicky showed 16 running instances on my machine (the exact scenario from Multiple Finicky instances accumulate in menu bar (no single-instance guard) #515).
  • After open apps/finicky/build/Finicky-test.app, only 1 process remains — the new one.
  • log show --predicate 'processImagePath CONTAINS "Finicky-test"' shows 16 Terminating duplicate Finicky instance (pid N) lines, one per stale instance.
  • Menu bar returned to a single Finicky icon.

Notes

  • Normal startup cost is a single NSRunningApplication enumeration — negligible.
  • Edge case: two Finicky processes launched near-simultaneously would each try to terminate the other. This is extremely rare in practice (URL events don't typically fire that close together), and the worst-case outcome is that both die and the next URL event relaunches a single fresh instance — no worse than the current behavior.
  • Bundle identifier is read from [[NSBundle mainBundle] bundleIdentifier] at runtime, so it stays correct if the bundle ID is ever renamed.

Summary by CodeRabbit

  • Bug Fixes
    • The app now ensures only one instance runs at a time on startup. Duplicate running instances are detected, logged, and asked to exit gracefully; the app waits briefly for them to close and will force terminate any remaining duplicates before proceeding. This prevents multiple copies from running concurrently and avoids startup conflicts.

macOS Launch Services normally routes GetURL events to an already-running
Finicky instance, but that routing can fail (stale LS registration after
app moves/updates, different bundle paths, SSO agents launching from
another context) and silently spawn a second process that proceeds to
create its own status bar icon. Without a guard, duplicates accumulate
in the menu bar over time.

Enumerate running applications with our bundle identifier at launch and
terminate any other instances so we're the single surviving process.

Verified locally: launching the patched build with 16 stale instances
already running reduced the count to 1 (the new process), with NSLog
entries confirming each termination.

Fixes johnste#515
@coderabbitai

coderabbitai Bot commented Apr 17, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 77009a9f-0505-47ef-a314-f50bc5911e79

📥 Commits

Reviewing files that changed from the base of the PR and between 32d9c4d and f905de4.

📒 Files selected for processing (1)
  • apps/finicky/src/main.m
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/finicky/src/main.m

📝 Walkthrough

Walkthrough

Added a runtime single-instance guard: on launch the app resolves its bundle ID, enumerates other running instances (excluding itself), logs and requests graceful termination, waits briefly while spinning the run loop, then force-terminates any lingering duplicates before continuing startup.

Changes

Cohort / File(s) Summary
Single-Instance Guard
apps/finicky/src/main.m
Added - (void)terminateOtherInstances and call from applicationDidFinishLaunching:. Method resolves bundle identifier, finds NSRunningApplication duplicates (excluding current PID), logs them, calls terminate, waits ~1s while running the run loop, then logs and forceTerminates any remaining instances.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Poem

🐇 I hopped in at sunrise bright,

chased twin shadows out of sight,
a gentle nudge, a final thump—adieu,
now one small app hops steady and true.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main change: terminating duplicate Finicky instances at launch, and references the resolved issue (#515).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/finicky/src/main.m`:
- Around line 69-72: Replace the current immediate-check logic around [app
terminate] so that after sending the terminate request (the NSLog and [app
terminate] call in the duplicate-instance block) you poll the
NSRunningApplication instance (app) for isTerminated with a short, bounded wait
(e.g., loop with small sleep and overall timeout of a few seconds); if the
process is still alive after the timeout call [app forceTerminate]. Keep the
original NSLog for pid ([app processIdentifier]) and ensure you only skip
forceTerminate when isTerminated becomes true within the timeout window.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3aac186c-f465-4e23-beca-696dcf57fb3d

📥 Commits

Reviewing files that changed from the base of the PR and between 08eb1d3 and 32d9c4d.

📒 Files selected for processing (1)
  • apps/finicky/src/main.m

Comment thread apps/finicky/src/main.m
[NSRunningApplication terminate] returning YES only confirms the request
was dispatched; the target may still be running when the call returns. If
a duplicate hung or ignored SIGTERM we'd reach createStatusItem alongside
it, defeating the single-instance guarantee.

Fire terminate() on all duplicates in parallel, then share a single 1s
deadline across the poll (so N hung duplicates don't each cost 1s), then
forceTerminate any holdouts.

Addresses review feedback on johnste#516.
@joeflateau

Copy link
Copy Markdown
Contributor Author

Good call from CodeRabbit — -[NSRunningApplication terminate] returning YES only confirms the request was sent, not that the target exited. Addressed in f905de4 with one change from the suggested fix: rather than waiting per-duplicate (which would cost N×deadline if many were hung), I fire all terminate() calls first, then share a single 1s deadline across the poll, then forceTerminate any holdouts.

Re-verified: spawned 3 stock Finicky instances with open -n /Applications/Finicky.app, launched the patched build — all 3 terminated cleanly via SIGTERM (no forceTerminate path hit), single instance remains.

@johnste johnste left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok to me! Thanks!

@johnste
johnste merged commit 1cb64ad into johnste:main Apr 18, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants