Skip to content

fix(hook): break the macOS run loop on a stop flag, not CFRunLoopStop alone#263

Merged
AprilNEA merged 1 commit into
masterfrom
fix/macos-hook-stop-race
Jul 18, 2026
Merged

fix(hook): break the macOS run loop on a stop flag, not CFRunLoopStop alone#263
AprilNEA merged 1 commit into
masterfrom
fix/macos-hook-stop-race

Conversation

@AprilNEA

Copy link
Copy Markdown
Owner

Problem

macos::stop() relies solely on CFRunLoopStop to terminate the hook thread. But CFRunLoopStop is a no-op unless the run loop is currently running: it only sets the stop flag when rl->_currentMode is non-null. The hook thread services the tap in 500 ms run_in_mode slices and, between slices, runs has_accessibility() (a TCC IPC) and tap.enable().

If stop() fires in that between-slices gap, the CF stop is dropped, nothing records the request, the thread re-enters a fresh 500 ms slice, and stop()'s thread.join() blocks forever — with the CGEventTap still live at the HID location. The window is small (a few ms out of every 500 ms) but the failure mode is a permanent hang on hook teardown.

Fix

Add an AtomicBool stop flag, checked at the top of every run-loop slice — mirroring the Linux backend's existing stop-flag pattern. stop() now sets the flag before calling run_loop.stop(), so shutdown always terminates:

  • common case (thread inside run_in_mode): run_loop.stop() wakes it immediately → returns Stopped → breaks, ~0 ms;
  • race case (stop lands between slices, CF stop dropped): the thread observes the flag at the next slice top → breaks within one 500 ms slice.

Either way the loop exits and falls through to the existing disable_tap(), so the tap is always detached on teardown. Relaxed ordering suffices: the flag carries no other data, and thread.join() provides the final synchronisation.

Testing

  • cargo clippy -p openlogi-hook --all-targets -- -D warnings — clean.
  • Full-workspace clippy via the pre-push hook — clean.

Not unit-tested: the path is FFI/timing-bound (a real repro needs Accessibility + a live CGEventTap), and the crate has no macOS unit tests today. A flag-only assertion would be tautological. The change is small and verifiable by inspection; happy to extract the slice-loop's termination decision behind a seam if a regression test is wanted.

@greptile-apps

greptile-apps Bot commented Jun 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a race-condition hang in the macOS hook teardown path by adding an AtomicBool stop flag to HookInner, checked at the top of every run_in_mode slice in thread_main. Previously, stop() relied solely on CFRunLoopStop, which is silently dropped when _currentMode is NULL (i.e., between slices), causing thread.join() to block indefinitely with the CGEventTap still live.

  • Adds stop: Arc<AtomicBool> to HookInner, threaded through startthread_main; stop() now stores true before calling run_loop.stop(), guaranteeing one of two exit paths is always taken.
  • Relaxed ordering is correct: the flag carries no other data, and thread.join() provides the final synchronisation barrier.
  • disable_tap is still reached on every exit path (normal and race), so the tap is always detached on teardown.

Confidence Score: 5/5

The change is a targeted, well-reasoned shutdown fix; all exit paths still reach disable_tap, and the AtomicBool pattern mirrors the Linux backend. Safe to merge.

The fix is minimal and correct: the stop flag is set before the CF wakeup, checked at every loop iteration top, and the existing disable_tap call on every exit path is preserved. No new logic branches are introduced that could cause regressions.

No files require special attention beyond the single changed file.

Important Files Changed

Filename Overview
crates/openlogi-hook/src/macos.rs Adds an AtomicBool stop flag to fix a CFRunLoopStop race that caused permanent hang on hook teardown; flag is checked at top of every slice and set before calling run_loop.stop(). Logic is correct, ordering is appropriate, and disable_tap remains on all exit paths.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller as stop() caller
    participant Inner as HookInner
    participant Thread as hook thread
    participant CF as CFRunLoop

    Note over Thread: loop top: check stop flag (false)
    Thread->>CF: run_in_mode(500ms)

    alt Normal case: stop() fires inside run_in_mode
        Caller->>Inner: stop.store(true, Relaxed)
        Caller->>CF: run_loop.stop()
        CF-->>Thread: returns Stopped
        Thread->>Thread: break → disable_tap()
        Caller->>Thread: thread.join() ✓
    else Race case: stop() fires between slices
        CF-->>Thread: returns TimedOut/HandledSource
        Thread->>Thread: has_accessibility() + tap.enable()
        Caller->>Inner: stop.store(true, Relaxed)
        Caller->>CF: run_loop.stop() — no-op (_currentMode is NULL)
        Thread->>Thread: loop top: check stop flag (true) → break → disable_tap()
        Caller->>Thread: thread.join() ✓ (≤500ms + IPC overhead)
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller as stop() caller
    participant Inner as HookInner
    participant Thread as hook thread
    participant CF as CFRunLoop

    Note over Thread: loop top: check stop flag (false)
    Thread->>CF: run_in_mode(500ms)

    alt Normal case: stop() fires inside run_in_mode
        Caller->>Inner: stop.store(true, Relaxed)
        Caller->>CF: run_loop.stop()
        CF-->>Thread: returns Stopped
        Thread->>Thread: break → disable_tap()
        Caller->>Thread: thread.join() ✓
    else Race case: stop() fires between slices
        CF-->>Thread: returns TimedOut/HandledSource
        Thread->>Thread: has_accessibility() + tap.enable()
        Caller->>Inner: stop.store(true, Relaxed)
        Caller->>CF: run_loop.stop() — no-op (_currentMode is NULL)
        Thread->>Thread: loop top: check stop flag (true) → break → disable_tap()
        Caller->>Thread: thread.join() ✓ (≤500ms + IPC overhead)
    end
Loading

Reviews (2): Last reviewed commit: "fix(hook): break the macOS run loop on a..." | Re-trigger Greptile

… alone

CFRunLoopStop is a no-op when it lands in the gap between the hook
thread's 500ms run_in_mode slices: the shutdown signal is dropped, the
loop re-enters a fresh slice, and stop()'s thread.join() blocks forever
with the event tap still live at the HID location.

Add an AtomicBool checked at the top of every slice. stop() now sets the
flag before calling run_loop.stop(), so shutdown always terminates:
promptly via the CF stop while a slice is running, otherwise within one
500ms slice via the flag. Mirrors the Linux backend's stop-flag pattern.
@AprilNEA
AprilNEA force-pushed the fix/macos-hook-stop-race branch from 4ecfed4 to e0fd699 Compare July 18, 2026 07:53
@AprilNEA
AprilNEA merged commit 3f80886 into master Jul 18, 2026
12 checks passed
@AprilNEA
AprilNEA deleted the fix/macos-hook-stop-race branch July 18, 2026 08:24
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