Repository navigation
fix: drop apps that quit, and reattach apps whose actor gave up - #641
Open
joonhoekim wants to merge 2 commits into
Open
joonhoekim wants to merge 2 commits into
joonhoekim wants to merge 2 commits into
Conversation
By the time kEventAppTerminated arrives, GetProcessPID no longer resolves the terminated process's serial number, so the event was dropped without a word and no app was ever terminated: a quit app stayed in rift's list (rift-cli query applications) with its app actor and AX observer, for TextEdit after Quit as for an app killed with SIGTERM. When a termination's pid cannot be read, each active app that NSRunningApplication no longer reports as running is now terminated as AppTerminated would.
An app busy when its actor started refused AX requests for the two seconds init waits, and the actor exited. Only activating the app reattached it, and its windows stayed where the last rift had parked them, off screen and out of the overview, so nothing showed it to activate. Seen with Chrome and Brave after a restart: absent from rift-cli query applications for hours. AppLifecycle::sync now matches actors to the running apps in both directions: running regular apps with no actor are attached, and actors whose app no longer runs are terminated, the check AppsMayHaveTerminated already made. It runs on app activation and deactivation and on space state updates, at most once a second, and when a retry falls due: an actor that gives up while its app runs is retried after 2 seconds, doubling up to 5 minutes. An actor marks its handle once init succeeds, so a failure after it had attached starts the retries over.
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
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.
Problem
Two ways rift loses track of an app, both silently.
kEventAppTerminatedarrives,GetProcessPIDno longer resolves the terminated process's serial number,so the event is dropped. The app stays in
rift-cli query applicationswith its actor and AX observer: TextEdit after Quit, or any app killed with
SIGTERM.
initwaits twoseconds to register the app's AX notifications. If the app refuses AX
requests for that long, the actor exits, and only activating the app
(
retry_unattached_app) reattaches it. After a rift restart, the app'swindows in other workspaces are still where the previous rift parked them,
off screen and out of the overview, so nothing shows them to activate. Seen
with Chrome and Brave after a restart: missing from
query applicationsfor hours until activated. The failed
initis inferred (logging was at thedefault level); this change logs
Could not attach a running app; trying again.Reproduce
rift-cli query applicationsstill lists it.browser has windows in a workspace that is not shown. When its actor gives
up, the app never shows up in
query applications.Fix
NSRunningApplicationno longer reports as running is terminated asAppTerminatedwould terminate it (WmEvent::AppsMayHaveTerminated).AppLifecycle::syncmatches actors to the running apps in bothdirections: running regular apps with no actor are attached, and actors
whose app no longer runs are terminated (the check from commit 1). It runs
on app activation, deactivation and space state updates (at most once a
second), and when a retry falls due: an actor that gives up while its app
runs is retried after 2 s, doubling up to 5 min.
Testing
wm_controller.rs(app_lifecycle_tests).cargo test --lib: 888 passed. The 2 failures also fail onmain(fixedseparately in test: keep the window server's Space window lists out of unit tests #640).
cargo +nightly fmt --all --check.query applications. I could not make an app refuse AX on demand, so theretry path itself is covered by the unit tests only.