Skip to content

Spacing: restart every app on Apply#923

Open
heyalchang wants to merge 1 commit into
jordanbaird:macos-26from
heyalchang:fix/spacing-apply-all-apps
Open

Spacing: restart every app on Apply#923
heyalchang wants to merge 1 commit into
jordanbaird:macos-26from
heyalchang:fix/spacing-apply-all-apps

Conversation

@heyalchang
Copy link
Copy Markdown

@heyalchang heyalchang commented Apr 18, 2026

Fixes so that Apply in Menu Bar Item Spacing actually restarts every app (best efforts)

Menu Bar Item spacing is owned by each of the menu items and control center.
When Ice wants to change the spacing, all the apps need a restart to pick it up.

Three issues have been there for quite some time and made this non-deterministic.
Made it look like Apply didn't do anything.

Tested as below for various spacing width and app selection.

AL

  1. The per-PID loop used break instead of continue, so the first PID that hit a guard (Control Center, self, a stale process) skipped all the rest. Set iteration order is random, which is why Apply looked like it was hitting a random subset each time. Switched to continue and split the guard so each skip gets logged.

  2. signalAppToQuit could resume its continuation twice if the KVO observer and the timeout fired close together, which crashes. Guarded both resume paths with an unfair lock so resume happens at most once, plus a failsafe resume in the timeout path so the caller can't hang.

  3. launchApp skipped relaunching when an app with the target bundle id was already in runningApplications. Auto-respawning helpers (Login Items, launchd, Dropbox Helper, Spotlight) often spawn a new instance before we get there, so we'd see the respawn and bail without launching. Now we pass the pre-quit PID and exclude it from the check, so a fresh respawn under a new PID counts as fresh.

Tested on macOS 26.4.1. Apply at offset -4 relaunches all 13 third-party menu bar apps in my bar (Hazel, AlDente, 1Password, TunnelBear, Pastebot, Shottr, etc.). Offset 0 fully un-squishes. Not tested on multi-display.

…d PID

Three fixes to MenuBarItemSpacingManager:

1. Replace `break` with `continue` in the per-PID guard inside
   applyOffset's TaskGroup. The previous code exited the entire
   loop on the first PID that failed the guard (Control Center,
   self, or a stale NSRunningApplication), so a random subset of
   menu bar apps was silently skipped on every Apply. Also splits
   the compound guard into separate guards so the skip reason is
   logged at debug level, which makes Apply behavior on macOS 26
   debuggable.

2. Wrap `continuation.resume()` in `signalAppToQuit` with an
   `OSAllocatedUnfairLock<Bool>` and a new `tryClaimOnce()`
   extension. If the force-terminate path and the KVO observer
   fire close together, both callers can race to call resume on
   the same continuation, which traps. The lock makes resume
   exactly once. Also adds a failsafe resume in the timeout task
   so callers don't hang if KVO never fires after force terminate.

3. Pass the just-terminated PID into `launchApp` and ignore that
   PID in the "already running" lookup. Auto-respawning helpers
   (Login Items, launchd-managed agents) often spawn under a new
   PID before our launch decision, and the previous check would
   see the auto-respawn and return without launching. With the
   replaced-PID exclusion, an auto-respawn under a new PID is
   recognized as fresh, while the dead PID falls through to
   `openApplication(at:)` (a no-op when an instance already
   exists and `createsNewApplicationInstance = false`).
emindeniz99 pushed a commit to emindeniz99/Ice that referenced this pull request Apr 23, 2026
Apply ac/sourcetree's three fixes to MenuBarItemSpacingManager
verbatim from the upstream patch:

1. applyOffset's per-PID guard uses continue instead of break, so
   hitting Control Center / the Ice process / a stale
   NSRunningApplication no longer silently aborts the whole apply.
   Each skip also logs the reason at debug level.

2. signalAppToQuit's continuation is guarded with a
   tryClaimOnce()-style OSAllocatedUnfairLock<Bool> helper so the
   force-terminate failsafe and the KVO observer can't both call
   resume and trap. A 1-second failsafe inside the timeout task
   makes sure we resume even if KVO never fires post-terminate.

3. launchApp takes a replacedPID and excludes it from the
   'already running' check, so auto-respawning helpers (Login
   Items, launchd-managed agents) that come back under a new PID
   are still recognized as fresh rather than matching the dead
   process.
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.

1 participant