From b3d05b4ead0fe48c0e1b83fa6ca2bddc4e29f21b Mon Sep 17 00:00:00 2001 From: joonhoekim <26rote@gmail.com> Date: Tue, 20 Oct 2026 03:36:19 +0900 Subject: [PATCH 1/2] fix: drop apps that quit, whose pid Carbon can no longer read 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. --- src/actor/process.rs | 10 +++++++++- src/actor/wm_controller.rs | 41 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/actor/process.rs b/src/actor/process.rs index f5df2af81..ab0c4c624 100644 --- a/src/actor/process.rs +++ b/src/actor/process.rs @@ -88,7 +88,15 @@ impl ProcessActor { let pid = match unsafe { pid_for_psn(&psn) } { Some(pid) => pid, - None => return NO_ERR, + None => { + // A terminated process's PSN no longer resolves to a pid by + // the time the event arrives; look for the apps that ended. + if ev.kind == K_EVENT_APP_TERMINATED { + debug!("Carbon: App terminated (pid unknown)"); + wm_sender.send(WmEvent::AppsMayHaveTerminated); + } + return NO_ERR; + } }; match ev.kind { diff --git a/src/actor/wm_controller.rs b/src/actor/wm_controller.rs index ac230e75b..186736863 100644 --- a/src/actor/wm_controller.rs +++ b/src/actor/wm_controller.rs @@ -53,6 +53,9 @@ pub enum WmEvent { AppGloballyActivated(pid_t), AppGloballyDeactivated(pid_t), AppTerminated(pid_t), + /// An app terminated whose pid could not be read: each tracked app that is + /// no longer running is treated as terminated. + AppsMayHaveTerminated, AppExited(pid_t, AppThreadHandle), SpaceStateUpdated(ForwardedSpaceState, CoordinateConverter), PowerStateChanged(bool), @@ -234,6 +237,18 @@ impl AppLifecycle { Some((handle, rx)) } + /// The active apps for which `running` is false. + fn ended(&self, running: impl Fn(pid_t) -> bool) -> Vec { + let mut pids: Vec = self + .0 + .iter() + .filter(|(pid, (_, phase))| matches!(phase, AppPhase::Active) && !running(**pid)) + .map(|(pid, _)| *pid) + .collect(); + pids.sort_unstable(); + pids + } + fn terminate(&mut self, pid: pid_t) { if let Some((handle, phase)) = self.0.get_mut(&pid) { *phase = AppPhase::Stopping(None); @@ -362,6 +377,17 @@ impl WmController { sys::app::remove_application_observer(pid); self.apps.terminate(pid); } + AppsMayHaveTerminated => { + let ended = self.apps.ended(|pid| { + NSRunningApplication::with_process_id(pid) + .is_some_and(|app| !app.isTerminated()) + }); + for pid in ended { + debug!(?pid, "App no longer running; terminating its actor"); + sys::app::remove_application_observer(pid); + self.apps.terminate(pid); + } + } AppExited(pid, handle) => { if let Some(relaunch) = self.apps.exited(pid, &handle) { self.events_tx.send(Event::AppActorExited(pid, handle)); @@ -647,6 +673,21 @@ mod app_lifecycle_tests { } } + #[test] + fn only_active_apps_that_stopped_running_have_ended() { + let mut apps = AppLifecycle::default(); + let info = || AppInfo { + bundle_id: None, + localized_name: None, + }; + for pid in [1, 2, 3] { + let _ = apps.reserve(pid, info(), AppDiscoverySource::Process); + } + apps.terminate(3); + assert_eq!(apps.ended(|pid| pid == 1), vec![2], "3 is stopping already"); + assert!(apps.ended(|_| true).is_empty()); + } + #[test] fn dedupe_failure_termination_and_reuse() { for terminated in [false, true] { From d5cb29e555dc979dc75162e8d0ca130e251c5fa6 Mon Sep 17 00:00:00 2001 From: joonhoekim <26rote@gmail.com> Date: Mon, 2 Nov 2026 20:34:12 +0900 Subject: [PATCH 2/2] fix: reattach a running app whose actor gave up 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. --- src/actor/app.rs | 11 ++ src/actor/wm_controller.rs | 263 ++++++++++++++++++++++++++++++++++--- src/sys/app.rs | 18 +++ 3 files changed, 273 insertions(+), 19 deletions(-) diff --git a/src/actor/app.rs b/src/actor/app.rs index 4b64dab99..5f3f4b6d2 100644 --- a/src/actor/app.rs +++ b/src/actor/app.rs @@ -6,6 +6,7 @@ use std::cell::RefCell; use std::fmt::Debug; use std::num::NonZeroU32; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, LazyLock, Mutex}; use std::thread; use std::time::{Duration, Instant}; @@ -301,6 +302,8 @@ pub struct AppThreadHandle { requests_tx: actor::Sender, interactive_frames: InteractiveFrameQueue, frame_authority: FrameAuthority, + /// Set once the actor has attached to its app, shared by every clone. + attached: Arc, } /// The topology accepted when this producer was created. Clones held by an old @@ -461,6 +464,7 @@ impl AppThreadHandle { requests_tx, interactive_frames: Default::default(), frame_authority: Default::default(), + attached: Default::default(), } } @@ -471,6 +475,7 @@ impl AppThreadHandle { requests_tx, interactive_frames: Default::default(), frame_authority: Default::default(), + attached: Default::default(), }, rx, ) @@ -482,6 +487,11 @@ impl AppThreadHandle { self.requests_tx.same_channel(&other.requests_tx) } + /// Whether the actor got through `init`: watching its app and holding its windows. + pub(crate) fn is_attached(&self) -> bool { self.attached.load(Ordering::Relaxed) } + + pub(crate) fn mark_attached(&self) { self.attached.store(true, Ordering::Relaxed) } + pub(crate) fn set_topology_authority( &mut self, guard: &crate::actor::spaces::TopologyGuard, @@ -1092,6 +1102,7 @@ impl State { if !self.init(handle.clone(), info) { return; } + handle.mark_attached(); let this = RefCell::new(self); // The raises channel is owned by State, so joining both tasks would keep diff --git a/src/actor/wm_controller.rs b/src/actor/wm_controller.rs index 186736863..fe3fdeb10 100644 --- a/src/actor/wm_controller.rs +++ b/src/actor/wm_controller.rs @@ -5,6 +5,7 @@ use std::borrow::Cow; use std::collections::HashMap; use std::path::PathBuf; +use std::time::{Duration, Instant}; use dispatchr::queue; use dispatchr::time::Time; @@ -48,6 +49,8 @@ use crate::{layout_engine as layout, sys}; #[derive(Debug)] pub enum WmEvent { DiscoverRunningApps, + /// Match the actors to the running apps (`AppLifecycle::sync`): an app's retry fell due. + SyncApps, AppEventsRegistered, AppLaunch(pid_t, AppInfo, AppDiscoverySource), AppGloballyActivated(pid_t), @@ -200,11 +203,43 @@ pub struct WmController { receiver: Receiver, sender: Sender, apps: AppLifecycle, + /// Whether `DiscoverRunningApps` has run: `sync_apps` attaches nothing + /// before it, since rift's app events must be registered first. + discovered: bool, + /// When `sync_apps` last ran. + synced_at: Option, } // Reserve before spawning; retain the reservation until AX resources are dropped. #[derive(Default)] -struct AppLifecycle(HashMap); +struct AppLifecycle { + actors: HashMap, + /// Running apps whose actor gave up, and when to try each again. + retries: HashMap, +} + +/// An app that refused AX requests while its actor started. +#[derive(Clone, Copy, Debug)] +struct Retry { + failures: u32, + at: Instant, +} + +/// The wait before attaching an app again after its first failure; it doubles +/// with each failure in a row, up to `RETRY_LONGEST`. +const RETRY_FIRST: Duration = Duration::from_secs(2); +const RETRY_LONGEST: Duration = Duration::from_secs(300); +/// `sync_apps` runs at most this often, except when a retry falls due or an +/// app may have terminated. +const SYNC_INTERVAL: Duration = Duration::from_secs(1); + +/// What `AppLifecycle::sync` found to do. +struct AppSync { + /// Running apps with no actor to attach. + attach: Vec<(pid_t, T)>, + /// Actors whose app no longer runs, to terminate. + ended: Vec, +} enum AppPhase { Active, // Includes initialization. @@ -218,7 +253,7 @@ impl AppLifecycle { info: AppInfo, source: AppDiscoverySource, ) -> Option<(AppThreadHandle, actor::Receiver)> { - if let Some((_, phase)) = self.0.get_mut(&pid) { + if let Some((_, phase)) = self.actors.get_mut(&pid) { if let AppPhase::Stopping(relaunch) = phase { let source = if relaunch .as_ref() @@ -233,14 +268,14 @@ impl AppLifecycle { return None; } let (handle, rx) = AppThreadHandle::channel(); - self.0.insert(pid, (handle.clone(), AppPhase::Active)); + self.actors.insert(pid, (handle.clone(), AppPhase::Active)); Some((handle, rx)) } /// The active apps for which `running` is false. fn ended(&self, running: impl Fn(pid_t) -> bool) -> Vec { let mut pids: Vec = self - .0 + .actors .iter() .filter(|(pid, (_, phase))| matches!(phase, AppPhase::Active) && !running(**pid)) .map(|(pid, _)| *pid) @@ -250,7 +285,7 @@ impl AppLifecycle { } fn terminate(&mut self, pid: pid_t) { - if let Some((handle, phase)) = self.0.get_mut(&pid) { + if let Some((handle, phase)) = self.actors.get_mut(&pid) { *phase = AppPhase::Stopping(None); _ = handle.send(Request::Terminate); } @@ -261,14 +296,68 @@ impl AppLifecycle { pid: pid_t, handle: &AppThreadHandle, ) -> Option> { - if !self.0.get(&pid)?.0.same_actor(handle) { + if !self.actors.get(&pid)?.0.same_actor(handle) { return None; } - Some(match self.0.remove(&pid)?.1 { + if handle.is_attached() { + // It attached; a later failure is a first failure. + self.retries.remove(&pid); + } + Some(match self.actors.remove(&pid)?.1 { AppPhase::Stopping(info) => info, _ => None, }) } + + fn is_attached(&self, pid: pid_t) -> bool { self.actors.contains_key(&pid) } + + /// Whether `handle` is `pid`'s actor and was not asked to stop: when it + /// exits, it gave up on its own. + fn is_active(&self, pid: pid_t, handle: &AppThreadHandle) -> bool { + self.actors + .get(&pid) + .is_some_and(|(h, phase)| h.same_actor(handle) && matches!(phase, AppPhase::Active)) + } + + /// Note that `pid`'s actor gave up at `now` while the app ran on, and + /// return how long to wait before attaching it again. + fn attach_failed(&mut self, pid: pid_t, now: Instant) -> Duration { + let failures = self.retries.get(&pid).map_or(0, |retry| retry.failures) + 1; + let delay = RETRY_FIRST.saturating_mul(1 << (failures - 1).min(16)).min(RETRY_LONGEST); + self.retries.insert(pid, Retry { failures, at: now + delay }); + delay + } + + /// Match the actors to the running apps. Rift learns of apps from one-time + /// events (a launch, a termination, a window appearing), and an app that + /// misses one stays as it was: an app whose actor gave up while the app + /// was busy keeps its windows wherever the last rift left them, unmanaged. + /// Comparing what runs with what is attached finds it, whatever was missed. + /// + /// `attachable` are the running apps an actor may attach to; those with + /// none are attached, apart from rift itself and apps whose retry is not + /// due at `now`. Actors whose process is not `running` have ended. + fn sync( + &mut self, + attachable: Vec<(pid_t, T)>, + running: impl Fn(pid_t) -> bool, + own: pid_t, + now: Instant, + ) -> AppSync { + self.retries.retain(|pid, _| running(*pid)); + let attach = attachable + .into_iter() + .filter(|(pid, _)| { + *pid != own + && !self.is_attached(*pid) + && self.retries.get(pid).is_none_or(|retry| retry.at <= now) + }) + .collect(); + AppSync { + attach, + ended: self.ended(running), + } + } } impl WmController { @@ -297,6 +386,8 @@ impl WmController { receiver, sender: sender.clone(), apps: AppLifecycle::default(), + discovered: false, + synced_at: None, }; (this, sender) } @@ -316,6 +407,13 @@ impl WmController { use self::WmCommand::*; use self::WmEvent::*; + // Apps come and go, or the user turns to another app or Space: the + // moments to check that every running app has its actor. + let sync = matches!( + &event, + SpaceStateUpdated(..) | AppGloballyActivated(_) | AppGloballyDeactivated(_) + ); + match event { OverviewActivationTarget(target) => { if let Some(tx) = &self.mission_control_tx { @@ -361,7 +459,9 @@ impl WmController { for (pid, info) in sys::app::running_apps(None) { self.new_app(pid, info, AppDiscoverySource::Process); } + self.discovered = true; } + SyncApps => self.sync_apps(true), AppLaunch(pid, info, source) => { self.new_app(pid, info, source); } @@ -377,22 +477,21 @@ impl WmController { sys::app::remove_application_observer(pid); self.apps.terminate(pid); } - AppsMayHaveTerminated => { - let ended = self.apps.ended(|pid| { - NSRunningApplication::with_process_id(pid) - .is_some_and(|app| !app.isTerminated()) - }); - for pid in ended { - debug!(?pid, "App no longer running; terminating its actor"); - sys::app::remove_application_observer(pid); - self.apps.terminate(pid); - } - } + AppsMayHaveTerminated => self.sync_apps(true), AppExited(pid, handle) => { + let gave_up = self.apps.is_active(pid, &handle); if let Some(relaunch) = self.apps.exited(pid, &handle) { self.events_tx.send(Event::AppActorExited(pid, handle)); if let Some((info, source)) = relaunch { self.new_app(pid, info, source); + } else if gave_up && sys::app::is_running(pid) { + let delay = self.apps.attach_failed(pid, Instant::now()); + warn!(pid, ?delay, "Could not attach a running app; trying again"); + queue::main().after_f_s( + Time::new_after(Time::NOW, delay.as_nanos() as i64), + (self.sender.clone(), WmEvent::SyncApps), + |(sender, event)| sender.send(event), + ); } } } @@ -518,11 +617,44 @@ impl WmController { } } } + if sync { + self.sync_apps(false); + } + } + + /// Match the actors to the running apps (`AppLifecycle::sync`). `now` + /// runs it whenever called; otherwise at most once per `SYNC_INTERVAL`. + fn sync_apps(&mut self, now: bool) { + let at = Instant::now(); + if !now + && self + .synced_at + .is_some_and(|last| at.saturating_duration_since(last) < SYNC_INTERVAL) + { + return; + } + self.synced_at = Some(at); + let attachable = if self.discovered { + sys::app::regular_apps().collect() + } else { + Vec::new() + }; + let own = std::process::id() as pid_t; + let sync = self.apps.sync(attachable, sys::app::is_running, own, at); + for pid in sync.ended { + debug!(?pid, "App no longer running; terminating its actor"); + sys::app::remove_application_observer(pid); + self.apps.terminate(pid); + } + for (pid, info) in sync.attach { + info!(pid, bundle = ?info.bundle_id, "Attaching a running app that has no actor"); + self.new_app(pid, info, AppDiscoverySource::Process); + } } /// Attaching fails while an app refuses AX requests; retry when the user activates it. fn retry_unattached_app(&mut self, pid: pid_t) { - if self.apps.0.contains_key(&pid) { + if self.apps.is_attached(pid) { return; } let Some(running_app) = NSRunningApplication::with_process_id(pid) else { @@ -688,6 +820,99 @@ mod app_lifecycle_tests { assert!(apps.ended(|_| true).is_empty()); } + fn info() -> AppInfo { + AppInfo { + bundle_id: None, + localized_name: None, + } + } + + /// What `sync` attaches when `running` run, all of them attachable. + fn attach(apps: &mut AppLifecycle, running: &[pid_t], own: pid_t, now: Instant) -> Vec { + let attachable = running.iter().map(|&pid| (pid, ())).collect(); + let sync = apps.sync(attachable, |pid| running.contains(&pid), own, now); + sync.attach.into_iter().map(|(pid, _)| pid).collect() + } + + /// An app whose actor gave up while the app kept running is attached + /// again without being activated. + #[test] + fn an_app_that_could_not_be_attached_is_attached_again_without_activation() { + let own = 1; + let mut apps = AppLifecycle::default(); + let start = Instant::now(); + + let (handle, _rx) = apps.reserve(42, info(), AppDiscoverySource::Process).unwrap(); + assert!( + apps.is_active(42, &handle), + "it gave up; it was not asked to stop" + ); + assert!(apps.exited(42, &handle).is_some()); + let first = apps.attach_failed(42, start); + + assert!( + attach(&mut apps, &[own, 42], own, start + first / 2).is_empty(), + "not before its retry is due" + ); + assert_eq!(attach(&mut apps, &[own, 42], own, start + first), vec![42]); + + // Still busy: it waits longer before the next attempt. + let retried = start + first; + let (handle, _rx) = apps.reserve(42, info(), AppDiscoverySource::Process).unwrap(); + assert!(apps.exited(42, &handle).is_some()); + let second = apps.attach_failed(42, retried); + assert!(second > first); + assert!(attach(&mut apps, &[42], own, retried + first).is_empty()); + + // Attached at last: nothing more to do for it. + let (handle, _rx) = apps.reserve(42, info(), AppDiscoverySource::Process).unwrap(); + handle.mark_attached(); + assert!(attach(&mut apps, &[42], own, retried + second).is_empty()); + + // Should that actor end while the app runs, it starts over from the first wait. + assert!(apps.exited(42, &handle).is_some()); + assert_eq!(apps.attach_failed(42, retried + second), first); + } + + #[test] + fn running_apps_with_no_actor_are_attached() { + let own = 1; + let mut apps = AppLifecycle::default(); + let now = Instant::now(); + // 7's launch was missed; 8 has its actor; rift itself is never attached. + let _ = apps.reserve(8, info(), AppDiscoverySource::Process); + assert_eq!(attach(&mut apps, &[own, 7, 8], own, now), vec![7]); + + // An app asked to stop did not fail to attach. + let (handle, _rx) = apps.reserve(9, info(), AppDiscoverySource::Process).unwrap(); + apps.terminate(9); + assert!(!apps.is_active(9, &handle)); + + // A failed app that quit is forgotten, so its pid starts fresh if reused. + let (handle, _rx) = apps.reserve(10, info(), AppDiscoverySource::Process).unwrap(); + assert!(apps.exited(10, &handle).is_some()); + apps.attach_failed(10, now); + assert!(attach(&mut apps, &[], own, now).is_empty()); + assert_eq!(attach(&mut apps, &[10], own, now), vec![10]); + } + + #[test] + fn sync_ends_actors_whose_app_quit_and_attaches_only_what_may_be() { + let own = 1; + let mut apps = AppLifecycle::default(); + let now = Instant::now(); + let _ = apps.reserve(2, info(), AppDiscoverySource::Process); + let _ = apps.reserve(3, info(), AppDiscoverySource::WindowServer); + // 2 quit; 3 runs though not attachable (an accessory app found by its + // window); 4 runs and may be attached. + let sync = apps.sync(vec![(4, ())], |pid| pid == 3 || pid == 4, own, now); + assert_eq!(sync.ended, vec![2]); + assert_eq!( + sync.attach.into_iter().map(|(pid, _)| pid).collect::>(), + vec![4] + ); + } + #[test] fn dedupe_failure_termination_and_reuse() { for terminated in [false, true] { diff --git a/src/sys/app.rs b/src/sys/app.rs index e061205ed..7062ae005 100644 --- a/src/sys/app.rs +++ b/src/sys/app.rs @@ -354,6 +354,24 @@ pub fn running_apps(bundle: Option) -> impl Iterator impl Iterator { + NSWorkspace::sharedWorkspace() + .runningApplications() + .into_iter() + .filter_map(|app| { + (app.activationPolicy() == NSApplicationActivationPolicy::Regular + && app.isFinishedLaunching() + && !app.isTerminated()) + .then(|| (app.pid(), AppInfo::from(&*app))) + }) +} + +pub fn is_running(pid: pid_t) -> bool { + NSRunningApplication::with_process_id(pid).is_some_and(|app| !app.isTerminated()) +} + /// Check one bundle identity without producing a process inventory. /// /// Persistence uses this only to decide whether a stale saved window still has a running