fix(hook): keep the short budget for tap teardown after a probe break - #1600
Open
TheMedpreneur wants to merge 4 commits into
Open
TheMedpreneur wants to merge 4 commits into
TheMedpreneur wants to merge 4 commits into
Conversation
…budget `service_tap` marks tap progress immediately before and immediately after its 500 ms `CFRunLoopRunInMode` slice, then runs the between-slice capability probe — `AXIsProcessTrusted()`, the throwaway `CGEventTapCreate` in `can_filter_events()`, `CGEventTapIsEnabled`, `CGEventTapEnable` — without marking again until the top of the next iteration. That window is charged to `TAP_SHUTDOWN_BUDGET`, so a probe slower than 1.5 s reads as a wedged tap thread and the lifecycle watchdog force-exits the agent with `FREEZE_HAZARD_EXIT_CODE`. Those calls are WindowServer and TCC round trips, and while the display is asleep they take seconds. A user's agent log shows the resulting exit repeat 27 times across 2.5 h of display sleep, always `reason="HID tap thread stopped making progress while tap remained active"`, `phase=Armed`, `elapsed_ms` between 1555 and 1740, with launchd restarting the agent 4-8 s later each time. Since the run-loop slice is bounded at 500 ms and re-marks progress on both sides, the whole overrun is inside the probe. The tap thread now publishes `TapPhase::Probing` for that window and returns to `Armed` with a fresh progress mark once `CGEventTapEnable` returns. The watchdog judges `Probing` against a separate `TAP_PROBE_BUDGET` for both exit reasons, so a stop request landing on a slow probe is treated the same way; a thread that reached the `Probing` store is demonstrably alive and inside a named CoreGraphics/TCC call rather than wedged servicing the tap. Everything else keeps the 1.5 s budget, `Probing` stays hazardous for the re-check before exiting, and a probe that never returns — the TCC-revocation stall this watchdog exists for — still force-exits. The freeze exposure does not grow with the budget: an active tap whose thread is not servicing its run loop is already bounded by CoreGraphics' own tap timeout, which disables the tap and lets events through. The break paths deliberately leave `Probing` published; the thread is still inside CoreGraphics for the synchronous teardown, and the watchdog stays armed on that phase either way. Refs AprilNEA#952
Follow-up to the Probing phase: the revocation and re-arm-exhausted breaks left Probing published, so tearing down a tap that outlived its permission got the 10 s probe budget instead of 1.5 s. Return to Armed with a fresh progress mark before breaking.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…op clock on progress Review follow-up. Leaving Probing stored Armed before the fresh progress mark, so a watchdog poll between the two stores could pair the short budget with the pre-probe timestamp and force-exit a healthy agent; a single resume_armed() now marks progress first at all three probe exits. A stop request landing during a slow probe kept measuring from the stop time, so teardown could start with its budget already spent; the stop deadline now runs from the later of the request and the last progress mark, which a wedged thread never advances.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacked on #1282. The first commit is #1282 unchanged; only the second commit is new.
In #1282, the revocation break and the re-arm-exhausted break leave
TapPhase::Probingpublished. Tearing down a tap that outlived its Accessibility grant is the freeze hazard this watchdog exists for, but it would then be judged against the 10 s probe budget instead of 1.5 s.Changes
macos.rs): return toArmedwith a fresh progress mark before both breaks, so teardown keeps the short budget.Testing
cargo fmt --all -- --check,cargo clippy -p openlogi-hook --all-targets -- -D warnings,cargo test -p openlogi-hook.phase=Armedforce-exits around wake and unlock. With it applied there have been 0 across 24 display/session resumes, including an overnight sleep. The revocation path is not runtime-tested.Refs #1282, #952