From 0b3b7dc6dc33c74f10bf791df2b2ea3578c375ac Mon Sep 17 00:00:00 2001 From: chall37 Date: Tue, 20 Jan 2026 21:47:04 -0800 Subject: [PATCH 1/9] Optimize iTermProcessCache with coalesced incremental refresh Major refactoring to reduce CPU usage from process monitoring with many tabs: - Add global coalescer using DISPATCH_SOURCE_TYPE_DATA_OR to merge concurrent monitor events into batched refresh cycles - Implement collection-first refresh that walks existing process collection instead of enumerating all PIDs via syscalls - Suspend monitors for background tabs; only foreground tabs get fast updates - Add epoch-based tracking to detect stale cache entries - Preserve dirty PID signaling for session title/status updates - Only remove cache entries when kill(pid,0) confirms process is dead, avoiding transient nil results from collection staleness Supporting changes: - iTermProcessMonitor: Add trackedRootPID property and pause/resume methods - iTermLSOF: Add childPidsForPid: using proc_listchildpids with retry loop - PseudoTerminal: Call setForegroundRootPIDs: on tab selection --- sources/PseudoTerminal.m | 12 + sources/iTermLSOF.h | 1 + sources/iTermLSOF.m | 44 ++++ sources/iTermProcessCache.h | 5 + sources/iTermProcessCache.m | 471 +++++++++++++++++++++++++++++++++- sources/iTermProcessMonitor.h | 19 +- sources/iTermProcessMonitor.m | 51 +++- 7 files changed, 591 insertions(+), 12 deletions(-) diff --git a/sources/PseudoTerminal.m b/sources/PseudoTerminal.m index 60dca1fae0..71d5dd8f48 100644 --- a/sources/PseudoTerminal.m +++ b/sources/PseudoTerminal.m @@ -49,6 +49,7 @@ #import "iTermOrderEnforcer.h" #import "iTermPasswordManagerWindowController.h" #import "iTermPreferences.h" +#import "iTermProcessCache.h" #import "iTermProfilePreferences.h" #import "iTermProfilesWindowController.h" #import "iTermPromptOnCloseReason.h" @@ -6704,6 +6705,17 @@ - (void)tabView:(NSTabView *)tabView didSelectTabViewItem:(NSTabViewItem *)tabVi [_contentView setCurrentSessionAlpha:self.currentSession.textview.transparencyAlpha]; [tab didSelectTab]; [[NSNotificationCenter defaultCenter] postNotificationName:iTermSelectedTabDidChange object:tab]; + + // Update process cache with foreground root PIDs for this window + NSMutableSet *foregroundPIDs = [NSMutableSet set]; + for (PTYSession *session in [tab sessions]) { + pid_t pid = session.shell.pid; + if (pid > 0) { + [foregroundPIDs addObject:@(pid)]; + } + } + [[iTermProcessCache sharedInstance] setForegroundRootPIDs:foregroundPIDs]; + DLog(@"Finished"); } diff --git a/sources/iTermLSOF.h b/sources/iTermLSOF.h index 3a21049a19..7fdb5cfaf6 100644 --- a/sources/iTermLSOF.h +++ b/sources/iTermLSOF.h @@ -19,6 +19,7 @@ int iTermProcPidInfoWrapper(int pid, int flavor, uint64_t arg, void *buffer, in + (NSString *)commandForProcess:(pid_t)pid execName:(NSString **)execName; + (NSArray *)allPids; + (pid_t)ppidForPid:(pid_t)childPid; ++ (NSArray *)childPidsForPid:(pid_t)parentPid; + (NSString *)nameOfProcessWithPid:(pid_t)thePid isForeground:(BOOL *)isForeground; + (NSString *)workingDirectoryOfProcess:(pid_t)pid; + (void)asyncWorkingDirectoryOfProcess:(pid_t)pid diff --git a/sources/iTermLSOF.m b/sources/iTermLSOF.m index c38884cbf3..16803c84c6 100644 --- a/sources/iTermLSOF.m +++ b/sources/iTermLSOF.m @@ -246,6 +246,50 @@ + (pid_t)ppidForPid:(pid_t)childPid { } } +// Returns direct child PIDs for a given parent PID. +// Note: proc_listchildpids returns PID count (not bytes like proc_listpids). ++ (NSArray *)childPidsForPid:(pid_t)parentPid { + int count = proc_listchildpids(parentPid, NULL, 0); + if (count <= 0) { + return @[]; + } + + pid_t *pids = NULL; + int returnedCount = 0; + + // Retry loop: child list can change between size query and fetch + for (int attempt = 0; attempt < 3; attempt++) { + int capacity = count + 16; // Headroom for new children + size_t bufferSize = capacity * sizeof(pid_t); + pids = (pid_t *)iTermMalloc(bufferSize); + + returnedCount = proc_listchildpids(parentPid, pids, (int)bufferSize); + if (returnedCount < 0) { + free(pids); + return @[]; + } + if (returnedCount <= capacity) { + break; + } + // Buffer too small, retry with larger capacity + free(pids); + pids = NULL; + count = returnedCount; + } + + // If all retries exhausted (child count kept growing faster than we could allocate), bail out + if (pids == NULL) { + return @[]; + } + + NSMutableArray *result = [NSMutableArray arrayWithCapacity:returnedCount]; + for (int i = 0; i < returnedCount; i++) { + [result addObject:@(pids[i])]; + } + free(pids); + return result; +} + // Use sysctl magic to get the name of a process and whether it is controlling // the tty. This code was adapted from ps, here: // http://opensource.apple.com/source/adv_cmds/adv_cmds-138.1/ps/ diff --git a/sources/iTermProcessCache.h b/sources/iTermProcessCache.h index 750c74f0ef..6805678b31 100644 --- a/sources/iTermProcessCache.h +++ b/sources/iTermProcessCache.h @@ -19,6 +19,11 @@ NS_ASSUME_NONNULL_BEGIN + (instancetype)sharedInstance; + (iTermProcessCollection *)newProcessCollection; +// Update which root PIDs are foreground (high priority). +// Foreground roots get fast incremental updates via process monitors. +// Background roots have their monitors suspended and rely on the 0.5s cadence. +- (void)setForegroundRootPIDs:(NSSet *)foregroundPIDs; + @end NS_ASSUME_NONNULL_END diff --git a/sources/iTermProcessCache.m b/sources/iTermProcessCache.m index e4d2017fd3..4548ba674c 100644 --- a/sources/iTermProcessCache.m +++ b/sources/iTermProcessCache.m @@ -16,6 +16,32 @@ #import "NSArray+iTerm.h" #import +// Event bits for coalescer +typedef NS_OPTIONS(unsigned long, iTermProcessCacheCoalescerEvent) { + iTermProcessCacheCoalescerEventHighPriority = 1 << 0, + iTermProcessCacheCoalescerEventLowPriority = 1 << 1, +}; + +// Per-root tracking with cached subtree and epoch +@interface iTermTrackedRootInfo : NSObject +@property (nonatomic) BOOL isHighPriority; +@property (nonatomic, strong, nullable) iTermProcessMonitor *monitor; // nil if suspended (background) +@property (nonatomic, strong) NSMutableIndexSet *cachedDescendants; // PIDs in last snapshot +@property (nonatomic) NSUInteger lastRefreshEpoch; +@property (nonatomic) BOOL isDirty; +@end + +@implementation iTermTrackedRootInfo +- (instancetype)init { + self = [super init]; + if (self) { + _cachedDescendants = [NSMutableIndexSet indexSet]; + _isHighPriority = YES; // Default to high priority (foreground) + } + return self; +} +@end + @interface iTermProcessCache() // Maps process id to deepest foreground job. _lockQueue @@ -27,11 +53,20 @@ @implementation iTermProcessCache { dispatch_queue_t _lockQueue; dispatch_queue_t _workQueue; iTermProcessCollection *_collectionLQ; // _lockQueue - NSMutableDictionary *_trackedPidsLQ; // _lockQueue + NSMutableDictionary *_trackedPidsLQ; // _lockQueue (legacy, being replaced by _rootsLQ) NSMutableArray *_blocksLQ; // _lockQueue BOOL _needsUpdateFlagLQ; // _lockQueue iTermRateLimitedUpdate *_rateLimit; // Main queue. keeps updateIfNeeded from eating all the CPU NSMutableIndexSet *_dirtyPIDsLQ; // _lockQueue + + // Per-root tracking (new coalescing system) + NSMutableDictionary *_rootsLQ; // _lockQueue + + // Global coalescer (dispatch_source DATA_OR) + dispatch_source_t _coalescer; // Merges all monitor events (_workQueue) + NSMutableIndexSet *_dirtyHighRootsLQ; // High-priority roots needing refresh (_lockQueue) + NSMutableIndexSet *_dirtyLowRootsLQ; // Low-priority (background) roots (_lockQueue) + NSUInteger _currentEpoch; // Incremented on each refresh cycle (_workQueue) } + (instancetype)sharedInstance { @@ -52,6 +87,15 @@ - (instancetype)init { _dirtyPIDsLQ = [NSMutableIndexSet indexSet]; _blocksLQ = [NSMutableArray array]; + // Initialize new coalescing system + _rootsLQ = [NSMutableDictionary dictionary]; + _dirtyHighRootsLQ = [NSMutableIndexSet indexSet]; + _dirtyLowRootsLQ = [NSMutableIndexSet indexSet]; + _currentEpoch = 0; + + // Set up global coalescer (DATA_OR merges concurrent events) + [self setupCoalescer]; + // I'm not fond of this pattern (code that sometimes is synchronous and sometimes not) but // I don't want to break -setNeedsUpdate when called on the main queue and that requires // synchronous initialization. Job managers use the process cache on their own queues and @@ -203,11 +247,14 @@ - (iTermProcessInfo *)deepestForegroundJobForPid:(pid_t)pid { - (void)registerTrackedPID:(pid_t)pid { dispatch_async(_lockQueue, ^{ __weak __typeof(self) weakSelf = self; + + // Create monitor with trackedRootPID for new coalescing system iTermProcessMonitor *monitor = [[iTermProcessMonitor alloc] initWithQueue:self->_lockQueue - callback: - ^(iTermProcessMonitor * monitor, dispatch_source_proc_flags_t flags) { - [weakSelf processMonitor:monitor didChangeFlags:flags]; - }]; + callback:^(iTermProcessMonitor *mon, dispatch_source_proc_flags_t flags) { + [weakSelf processMonitor:mon didChangeFlags:flags]; + } + trackedRootPID:pid]; + iTermProcessInfo *info = [self->_collectionLQ infoForProcessID:pid]; if (!info) { DLog(@"Request update for %@", @(pid)); @@ -216,9 +263,22 @@ - (void)registerTrackedPID:(pid_t)pid { [weakSelf didUpdateForPid:pid]; }]; } else { - monitor.processInfo = info; + [monitor setProcessInfo:info]; } + + // Legacy tracking (for backward compatibility) self->_trackedPidsLQ[@(pid)] = monitor; + + // New coalescing system - create iTermTrackedRootInfo + iTermTrackedRootInfo *rootInfo = [[iTermTrackedRootInfo alloc] init]; + rootInfo.monitor = monitor; + rootInfo.isHighPriority = YES; // New registrations are foreground by default + rootInfo.isDirty = YES; + self->_rootsLQ[@(pid)] = rootInfo; + + // Trigger initial refresh for this root + [self->_dirtyHighRootsLQ addIndex:pid]; + dispatch_source_merge_data(self->_coalescer, iTermProcessCacheCoalescerEventHighPriority); }); } @@ -241,12 +301,36 @@ - (void)didUpdateForPid:(pid_t)pid { // lockQueue - (void)processMonitor:(iTermProcessMonitor *)monitor didChangeFlags:(dispatch_source_proc_flags_t)flags { DLog(@"Flags changed for %@.", @(monitor.processInfo.processID)); + + // New coalescing system: use trackedRootPID to find the root and signal coalescer + pid_t rootPID = monitor.trackedRootPID; + if (rootPID > 0) { + iTermTrackedRootInfo *rootInfo = _rootsLQ[@(rootPID)]; + if (rootInfo) { + rootInfo.isDirty = YES; + + // Preserve dirty PID signaling for processIsDirty: callers (e.g., session title updates) + [_dirtyPIDsLQ addIndex:rootPID]; + + if (rootInfo.isHighPriority) { + [_dirtyHighRootsLQ addIndex:rootPID]; + // Signal coalescer (merges with other concurrent events) + dispatch_source_merge_data(_coalescer, iTermProcessCacheCoalescerEventHighPriority); + } else { + // Background root - just mark dirty, will be handled by cadence timer + [_dirtyLowRootsLQ addIndex:rootPID]; + } + return; // New system handled it + } + } + + // Legacy path: fall back to old behavior for monitors not in the new system _needsUpdateFlagLQ = YES; const BOOL wasForced = self.forcingLQ; self.forcingLQ = YES; if (!wasForced) { dispatch_async(dispatch_get_main_queue(), ^{ - DLog(@"Forcing update"); + DLog(@"Forcing update (legacy path)"); [self->_rateLimit performRateLimitedSelector:@selector(updateIfNeeded) onTarget:self withObject:nil]; [self->_rateLimit performWithinDuration:0.0167]; self.forcingLQ = NO; @@ -270,7 +354,20 @@ - (BOOL)processIsDirty:(pid_t)pid { // Any queue - (void)unregisterTrackedPID:(pid_t)pid { dispatch_async(_lockQueue, ^{ + // Legacy cleanup [self->_trackedPidsLQ removeObjectForKey:@(pid)]; + + // New coalescing system cleanup + iTermTrackedRootInfo *rootInfo = self->_rootsLQ[@(pid)]; + if (rootInfo) { + // Invalidate the monitor (stops the dispatch source) + [rootInfo.monitor invalidate]; + [self->_rootsLQ removeObjectForKey:@(pid)]; + } + + // Remove from dirty sets + [self->_dirtyHighRootsLQ removeIndex:pid]; + [self->_dirtyLowRootsLQ removeIndex:pid]; }); } @@ -283,6 +380,10 @@ - (void)sendSignal:(int32_t)signal toPID:(int32_t)pid { // Any queue - (void)updateIfNeeded { DLog(@"updateIfNeeded"); + + // Always process background roots on the cadence timer (amortized refresh) + [self backgroundRefreshTick]; + __block BOOL needsUpdate; dispatch_sync(_lockQueue, ^{ needsUpdate = self->_needsUpdateFlagLQ; @@ -360,6 +461,362 @@ - (void)reallyUpdate { } } +#pragma mark - Coalescer + +- (void)setupCoalescer { + _coalescer = dispatch_source_create(DISPATCH_SOURCE_TYPE_DATA_OR, 0, 0, _workQueue); + + __weak __typeof(self) weakSelf = self; + dispatch_source_set_event_handler(_coalescer, ^{ + [weakSelf handleCoalescedEvents]; + }); + dispatch_resume(_coalescer); +} + +// _workQueue +- (void)handleCoalescedEvents { + unsigned long data = dispatch_source_get_data(_coalescer); + BOOL hasHighPriority = (data & iTermProcessCacheCoalescerEventHighPriority) != 0; + + if (hasHighPriority) { + // Immediate incremental refresh for dirty high-priority roots + [self refreshDirtyHighPriorityRoots]; + } + // Low-priority handled by 0.5s cadence timer (backgroundRefreshTick), not here +} + +// _workQueue +- (void)refreshDirtyHighPriorityRoots { + _currentEpoch++; + + __block NSMutableArray *dirtyRoots = [NSMutableArray array]; + __block iTermProcessCollection *collection; + + dispatch_sync(_lockQueue, ^{ + [self->_dirtyHighRootsLQ enumerateIndexesUsingBlock:^(NSUInteger pid, BOOL *stop) { + [dirtyRoots addObject:@(pid)]; + }]; + [self->_dirtyHighRootsLQ removeAllIndexes]; + collection = self->_collectionLQ; // Capture reference under lock + }); + + if (!collection || dirtyRoots.count == 0) { + return; + } + + NSMutableDictionary *newCache = [NSMutableDictionary dictionary]; + NSMutableArray *confirmedDeadRoots = [NSMutableArray array]; + + for (NSNumber *rootPidNum in dirtyRoots) { + pid_t rootPid = rootPidNum.intValue; + iTermProcessInfo *result = [self refreshRootCollectionFirst:rootPid + collection:collection + epoch:_currentEpoch]; + if (result) { + newCache[rootPidNum] = result; + } else { + // Refresh returned nil - check if process is actually dead before removing cache entry. + // This avoids dropping cached data due to collection staleness. + if (![self processIsAlive:rootPid]) { + [confirmedDeadRoots addObject:rootPidNum]; + } + // If process is still alive, preserve existing cache entry (don't add to newCache or deadRoots) + } + } + + dispatch_sync(_lockQueue, ^{ + // Merge new cache entries into existing cache + NSMutableDictionary *mutableCache = [self->_cachedDeepestForegroundJobLQ mutableCopy] ?: [NSMutableDictionary dictionary]; + [mutableCache addEntriesFromDictionary:newCache]; + + // Only remove entries for roots confirmed dead (not just fallback/stale) + for (NSNumber *deadPid in confirmedDeadRoots) { + [mutableCache removeObjectForKey:deadPid]; + } + + self->_cachedDeepestForegroundJobLQ = [mutableCache copy]; + + // Evict unseen nodes from per-root caches + for (NSNumber *rootPidNum in dirtyRoots) { + [self evictUnseenNodesForRoot:rootPidNum.intValue epoch:self->_currentEpoch]; + } + }); +} + +// _workQueue - Fast path: walk existing collection (no PID enumeration syscalls) +- (iTermProcessInfo *)refreshRootCollectionFirst:(pid_t)rootPid + collection:(iTermProcessCollection *)collection + epoch:(NSUInteger)epoch { + __block iTermTrackedRootInfo *rootInfo; + dispatch_sync(_lockQueue, ^{ + rootInfo = self->_rootsLQ[@(rootPid)]; + }); + + if (!rootInfo) { + return nil; + } + + iTermProcessInfo *root = [collection infoForProcessID:rootPid]; + if (!root) { + // Root disappeared - fall back to kernel query + return [self refreshRootWithKernelFallback:rootPid epoch:epoch]; + } + + // Walk foreground chain preferentially (root → fg child → fg grandchild) + iTermProcessInfo *candidate = root; + int depth = 0; + while (depth < 50) { + [self markSeenForEpoch:epoch pid:candidate.processID rootInfo:rootInfo]; + + iTermProcessInfo *fgChild = [self findForegroundChildOf:candidate inCollection:collection]; + if (!fgChild) { + break; + } + + // Verify child still exists in collection + if (![collection infoForProcessID:fgChild.processID]) { + // Inconsistency detected - bounded kernel fallback + return [self refreshRootWithKernelFallback:rootPid epoch:epoch]; + } + + candidate = fgChild; + depth++; + } + + dispatch_sync(_lockQueue, ^{ + rootInfo.isDirty = NO; + }); + + return candidate; // Deepest foreground job +} + +// _workQueue - Find foreground child using collection's cached foreground state +- (iTermProcessInfo *)findForegroundChildOf:(iTermProcessInfo *)parent + inCollection:(iTermProcessCollection *)collection { + // iTermProcessInfo has children and isForeground properties + for (iTermProcessInfo *child in parent.children) { + if (child.isForeground) { + return child; + } + } + return nil; +} + +// _workQueue - Fallback: bounded kernel queries for this root only +- (iTermProcessInfo *)refreshRootWithKernelFallback:(pid_t)rootPid epoch:(NSUInteger)epoch { + // Query kernel for root's immediate children only (bounded) + NSArray *children = [iTermLSOF childPidsForPid:rootPid]; + + __block iTermTrackedRootInfo *rootInfo; + dispatch_sync(_lockQueue, ^{ + rootInfo = self->_rootsLQ[@(rootPid)]; + }); + + if (!rootInfo) { + return nil; + } + + // Mark root as seen + [self markSeenForEpoch:epoch pid:rootPid rootInfo:rootInfo]; + + // For now, just mark children as seen and return nil (no deep traversal in fallback) + // The full refresh (reallyUpdate) will catch up on the next 0.5s tick + for (NSNumber *childPid in children) { + [self markSeenForEpoch:epoch pid:childPid.intValue rootInfo:rootInfo]; + } + + dispatch_sync(_lockQueue, ^{ + rootInfo.isDirty = NO; + }); + + return nil; // Let the regular cadence update provide the full answer +} + +// Thread-safe helper to mark a PID as seen in this epoch +- (void)markSeenForEpoch:(NSUInteger)epoch pid:(pid_t)pid rootInfo:(iTermTrackedRootInfo *)rootInfo { + dispatch_sync(_lockQueue, ^{ + [rootInfo.cachedDescendants addIndex:pid]; + rootInfo.lastRefreshEpoch = epoch; + }); +} + +// _lockQueue - Remove PIDs that weren't seen in the current epoch +- (void)evictUnseenNodesForRoot:(pid_t)rootPid epoch:(NSUInteger)epoch { + iTermTrackedRootInfo *info = _rootsLQ[@(rootPid)]; + if (!info) { + return; + } + + // If this root's last refresh epoch is older than current, it wasn't refreshed + // In that case, don't evict (it might be a background root) + if (info.lastRefreshEpoch < epoch) { + return; + } + + // For now, we don't track per-PID epochs, so we can't selectively evict + // The epoch system mainly prevents drift when roots become stale + // A more sophisticated implementation could track (pid -> lastSeenEpoch) in cachedDescendants +} + +// Check if a process is still alive using kill(pid, 0) +// Returns YES if process exists, NO if confirmed dead +- (BOOL)processIsAlive:(pid_t)pid { + if (pid <= 0) { + return NO; + } + int result = kill(pid, 0); + if (result == 0) { + return YES; // Process exists and we have permission to signal it + } + // Check errno: ESRCH means no such process, EPERM means exists but no permission + return (errno == EPERM); +} + +#pragma mark - Background Monitor Suspension + +// _lockQueue - Called when a tab loses foreground +- (void)suspendMonitorForRootLQ:(pid_t)rootPid { + iTermTrackedRootInfo *info = _rootsLQ[@(rootPid)]; + if (!info) { + return; + } + + DLog(@"Suspending monitor for root %@", @(rootPid)); + if (info.monitor) { + [info.monitor pauseMonitoring]; + } + info.isHighPriority = NO; + [_dirtyLowRootsLQ addIndex:rootPid]; +} + +// _lockQueue - Called when a tab becomes foreground +- (void)resumeMonitorForRootLQ:(pid_t)rootPid { + iTermTrackedRootInfo *info = _rootsLQ[@(rootPid)]; + if (!info) { + return; + } + + DLog(@"Resuming monitor for root %@", @(rootPid)); + info.isHighPriority = YES; + + if (info.monitor) { + [info.monitor resumeMonitoring]; + } else { + [self createMonitorForRootLQ:rootPid info:info]; + } + + // Immediate refresh for newly-foregrounded root + info.isDirty = YES; + [_dirtyHighRootsLQ addIndex:rootPid]; + dispatch_source_merge_data(_coalescer, iTermProcessCacheCoalescerEventHighPriority); +} + +// _lockQueue - Create a new monitor for a root PID +- (void)createMonitorForRootLQ:(pid_t)rootPid info:(iTermTrackedRootInfo *)info { + __weak __typeof(self) weakSelf = self; + iTermProcessMonitor *monitor = [[iTermProcessMonitor alloc] initWithQueue:_lockQueue + callback:^(iTermProcessMonitor *mon, dispatch_source_proc_flags_t flags) { + [weakSelf processMonitor:mon didChangeFlags:flags]; + } + trackedRootPID:rootPid]; + + iTermProcessInfo *processInfo = [_collectionLQ infoForProcessID:rootPid]; + if (processInfo) { + [monitor setProcessInfo:processInfo]; + } + + info.monitor = monitor; +} + +// Any queue - Public API for tab selection +- (void)setForegroundRootPIDs:(NSSet *)foregroundPIDs { + dispatch_async(_lockQueue, ^{ + [self setForegroundRootPIDsLQ:foregroundPIDs]; + }); +} + +// _lockQueue +- (void)setForegroundRootPIDsLQ:(NSSet *)foregroundPIDs { + for (NSNumber *pidNum in _rootsLQ) { + iTermTrackedRootInfo *info = _rootsLQ[pidNum]; + BOOL shouldBeForeground = [foregroundPIDs containsObject:pidNum]; + + if (shouldBeForeground && !info.isHighPriority) { + [self resumeMonitorForRootLQ:pidNum.intValue]; + } else if (!shouldBeForeground && info.isHighPriority) { + [self suspendMonitorForRootLQ:pidNum.intValue]; + } + } +} + +#pragma mark - Amortized Background Refresh + +// Called by 0.5s cadence timer - refresh 1-2 background roots per tick +- (void)backgroundRefreshTick { + __block NSArray *toRefresh = nil; + __block iTermProcessCollection *collection = nil; + __block NSUInteger epoch = 0; + + dispatch_sync(_lockQueue, ^{ + if (self->_dirtyLowRootsLQ.count == 0) { + return; + } + + NSMutableArray *picked = [NSMutableArray array]; + __block NSUInteger remaining = 2; // Process at most 2 background roots per tick + [self->_dirtyLowRootsLQ enumerateIndexesUsingBlock:^(NSUInteger pid, BOOL *stop) { + [picked addObject:@(pid)]; + if (--remaining == 0) { + *stop = YES; + } + }]; + + for (NSNumber *pidNum in picked) { + [self->_dirtyLowRootsLQ removeIndex:pidNum.unsignedIntegerValue]; + } + + toRefresh = [picked copy]; + collection = self->_collectionLQ; // Capture under lock + // Reuse current epoch (background refreshes don't advance epoch) + epoch = self->_currentEpoch; + }); + + if (toRefresh.count == 0) { + return; + } + + dispatch_async(_workQueue, ^{ + NSMutableArray *confirmedDeadRoots = [NSMutableArray array]; + NSMutableDictionary *newCache = [NSMutableDictionary dictionary]; + + for (NSNumber *pidNum in toRefresh) { + iTermProcessInfo *result = [self refreshRootCollectionFirst:pidNum.intValue + collection:collection + epoch:epoch]; + if (result) { + newCache[pidNum] = result; + } else { + // Only remove cache entry if process is confirmed dead + if (![self processIsAlive:pidNum.intValue]) { + [confirmedDeadRoots addObject:pidNum]; + } + } + } + + // Update cache with results and remove confirmed dead entries + if (newCache.count > 0 || confirmedDeadRoots.count > 0) { + dispatch_sync(self->_lockQueue, ^{ + NSMutableDictionary *mutableCache = [self->_cachedDeepestForegroundJobLQ mutableCopy] ?: [NSMutableDictionary dictionary]; + [mutableCache addEntriesFromDictionary:newCache]; + for (NSNumber *deadPid in confirmedDeadRoots) { + [mutableCache removeObjectForKey:deadPid]; + } + self->_cachedDeepestForegroundJobLQ = [mutableCache copy]; + }); + } + }); +} + #pragma mark - Notifications // Main queue diff --git a/sources/iTermProcessMonitor.h b/sources/iTermProcessMonitor.h index 9752b9a6b0..96132004f3 100644 --- a/sources/iTermProcessMonitor.h +++ b/sources/iTermProcessMonitor.h @@ -20,14 +20,29 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) dispatch_queue_t queue; @property (nullable, nonatomic, weak, readonly) iTermProcessMonitor *parent; +// The root PID that this monitor (or its ancestor) was created to track. +// Set at creation time and propagated to children for O(1) lookup in callbacks. +@property (nonatomic, readonly) pid_t trackedRootPID; + +- (instancetype)initWithQueue:(dispatch_queue_t)queue + callback:(void (^)(iTermProcessMonitor *, dispatch_source_proc_flags_t))callback + trackedRootPID:(pid_t)trackedRootPID NS_DESIGNATED_INITIALIZER; + +// Legacy initializer - creates a monitor without trackedRootPID (defaults to 0) - (instancetype)initWithQueue:(dispatch_queue_t)queue - callback:(void (^)(iTermProcessMonitor *, dispatch_source_proc_flags_t))callback NS_DESIGNATED_INITIALIZER; + callback:(void (^)(iTermProcessMonitor *, dispatch_source_proc_flags_t))callback; - (instancetype)init NS_UNAVAILABLE; -// Stops monitoring. +// Stops monitoring permanently. - (void)invalidate; +// Temporarily pauses monitoring (dispatch source suspended). Call resumeMonitoring to resume. +- (void)pauseMonitoring; + +// Resumes monitoring after pauseMonitoring was called. +- (void)resumeMonitoring; + - (void)addChild:(iTermProcessMonitor *)child; // Returns whether this or any child changed. Begins monitoring if nonnil. diff --git a/sources/iTermProcessMonitor.m b/sources/iTermProcessMonitor.m index 5374e2626e..86d70f4156 100644 --- a/sources/iTermProcessMonitor.m +++ b/sources/iTermProcessMonitor.m @@ -19,19 +19,27 @@ @interface iTermProcessMonitor() @implementation iTermProcessMonitor { dispatch_source_t _source; NSMutableArray *_children; + BOOL _isPaused; } - (instancetype)initWithQueue:(dispatch_queue_t)queue - callback:(void (^)(iTermProcessMonitor *, dispatch_source_proc_flags_t))callback { + callback:(void (^)(iTermProcessMonitor *, dispatch_source_proc_flags_t))callback + trackedRootPID:(pid_t)trackedRootPID { self = [super init]; if (self) { _callback = [callback copy]; _queue = queue; _children = [NSMutableArray array]; + _trackedRootPID = trackedRootPID; } return self; } +- (instancetype)initWithQueue:(dispatch_queue_t)queue + callback:(void (^)(iTermProcessMonitor *, dispatch_source_proc_flags_t))callback { + return [self initWithQueue:queue callback:callback trackedRootPID:0]; +} + - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo { return [self setProcessInfo:processInfo depth:0]; } @@ -92,8 +100,10 @@ - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo depth:(NSInteger)depth { return; } - // Create a new one. - child = [[iTermProcessMonitor alloc] initWithQueue:_queue callback:_callback]; + // Create a new one. Propagate trackedRootPID from parent. + child = [[iTermProcessMonitor alloc] initWithQueue:_queue + callback:_callback + trackedRootPID:_trackedRootPID]; [child setProcessInfo:childInfo depth:depth + 1]; [childrenToAdd addObject:child]; }]; @@ -132,6 +142,11 @@ - (void)invalidate { return; } DLog(@"Stop monitoring process %@", _processInfo); + // If paused, need to resume before canceling (dispatch sources must be resumed before cancel) + if (_isPaused) { + dispatch_resume(_source); + _isPaused = NO; + } dispatch_source_cancel(_source); _source = nil; [_parent removeChild:self]; @@ -143,6 +158,36 @@ - (void)invalidate { _processInfo = nil; } +// Called on _queue +- (void)pauseMonitoring { + if (_source == nil || _isPaused) { + return; + } + DLog(@"Pause monitoring process %@", _processInfo); + dispatch_suspend(_source); + _isPaused = YES; + + // Recursively pause children + for (iTermProcessMonitor *child in _children) { + [child pauseMonitoring]; + } +} + +// Called on _queue +- (void)resumeMonitoring { + if (_source == nil || !_isPaused) { + return; + } + DLog(@"Resume monitoring process %@", _processInfo); + dispatch_resume(_source); + _isPaused = NO; + + // Recursively resume children + for (iTermProcessMonitor *child in _children) { + [child resumeMonitoring]; + } +} + // Called on _queue - (void)addChild:(iTermProcessMonitor *)child { [_children addObject:child]; From d43483b92215dcf360e9c8ae287b422ac65061c1 Mon Sep 17 00:00:00 2001 From: chall37 Date: Tue, 20 Jan 2026 23:03:38 -0800 Subject: [PATCH 2/9] Fix performance issues in process cache incremental refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Throttle backgroundRefreshTick to 0.5s minimum intervals instead of running on every updateIfNeeded call - Batch dispatch_sync calls in refreshRootCollectionFirst: collect PIDs in NSMutableIndexSet, single sync at end instead of per-PID - Same batching fix for refreshRootWithKernelFallback - Remove unused markSeenForEpoch helper method - Fix property name: isForeground → isForegroundJob --- sources/iTermProcessCache.m | 43 +++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/sources/iTermProcessCache.m b/sources/iTermProcessCache.m index 4548ba674c..40b3f34a23 100644 --- a/sources/iTermProcessCache.m +++ b/sources/iTermProcessCache.m @@ -67,6 +67,9 @@ @implementation iTermProcessCache { NSMutableIndexSet *_dirtyHighRootsLQ; // High-priority roots needing refresh (_lockQueue) NSMutableIndexSet *_dirtyLowRootsLQ; // Low-priority (background) roots (_lockQueue) NSUInteger _currentEpoch; // Incremented on each refresh cycle (_workQueue) + + // Throttle for background refresh (ensures 0.5s minimum between background refreshes) + NSTimeInterval _lastBackgroundRefreshTime; } + (instancetype)sharedInstance { @@ -381,8 +384,12 @@ - (void)sendSignal:(int32_t)signal toPID:(int32_t)pid { - (void)updateIfNeeded { DLog(@"updateIfNeeded"); - // Always process background roots on the cadence timer (amortized refresh) - [self backgroundRefreshTick]; + // Process background roots only on ~0.5s cadence (not on every call) + NSTimeInterval now = [NSDate timeIntervalSinceReferenceDate]; + if (now - _lastBackgroundRefreshTime >= 0.5) { + _lastBackgroundRefreshTime = now; + [self backgroundRefreshTick]; + } __block BOOL needsUpdate; dispatch_sync(_lockQueue, ^{ @@ -563,10 +570,12 @@ - (iTermProcessInfo *)refreshRootCollectionFirst:(pid_t)rootPid } // Walk foreground chain preferentially (root → fg child → fg grandchild) + // Collect PIDs to mark as seen (batched to avoid per-PID dispatch_sync) + NSMutableIndexSet *seenPIDs = [NSMutableIndexSet indexSet]; iTermProcessInfo *candidate = root; int depth = 0; while (depth < 50) { - [self markSeenForEpoch:epoch pid:candidate.processID rootInfo:rootInfo]; + [seenPIDs addIndex:candidate.processID]; iTermProcessInfo *fgChild = [self findForegroundChildOf:candidate inCollection:collection]; if (!fgChild) { @@ -583,7 +592,10 @@ - (iTermProcessInfo *)refreshRootCollectionFirst:(pid_t)rootPid depth++; } + // Single dispatch_sync to mark all seen PIDs and clear dirty flag dispatch_sync(_lockQueue, ^{ + [rootInfo.cachedDescendants addIndexes:seenPIDs]; + rootInfo.lastRefreshEpoch = epoch; rootInfo.isDirty = NO; }); @@ -593,9 +605,9 @@ - (iTermProcessInfo *)refreshRootCollectionFirst:(pid_t)rootPid // _workQueue - Find foreground child using collection's cached foreground state - (iTermProcessInfo *)findForegroundChildOf:(iTermProcessInfo *)parent inCollection:(iTermProcessCollection *)collection { - // iTermProcessInfo has children and isForeground properties + // iTermProcessInfo has children and isForegroundJob properties for (iTermProcessInfo *child in parent.children) { - if (child.isForeground) { + if (child.isForegroundJob) { return child; } } @@ -616,30 +628,23 @@ - (iTermProcessInfo *)refreshRootWithKernelFallback:(pid_t)rootPid epoch:(NSUInt return nil; } - // Mark root as seen - [self markSeenForEpoch:epoch pid:rootPid rootInfo:rootInfo]; - - // For now, just mark children as seen and return nil (no deep traversal in fallback) - // The full refresh (reallyUpdate) will catch up on the next 0.5s tick + // Collect all PIDs to mark as seen (batched to avoid per-PID dispatch_sync) + NSMutableIndexSet *seenPIDs = [NSMutableIndexSet indexSet]; + [seenPIDs addIndex:rootPid]; for (NSNumber *childPid in children) { - [self markSeenForEpoch:epoch pid:childPid.intValue rootInfo:rootInfo]; + [seenPIDs addIndex:childPid.unsignedIntegerValue]; } + // Single dispatch_sync to mark all seen PIDs and clear dirty flag dispatch_sync(_lockQueue, ^{ + [rootInfo.cachedDescendants addIndexes:seenPIDs]; + rootInfo.lastRefreshEpoch = epoch; rootInfo.isDirty = NO; }); return nil; // Let the regular cadence update provide the full answer } -// Thread-safe helper to mark a PID as seen in this epoch -- (void)markSeenForEpoch:(NSUInteger)epoch pid:(pid_t)pid rootInfo:(iTermTrackedRootInfo *)rootInfo { - dispatch_sync(_lockQueue, ^{ - [rootInfo.cachedDescendants addIndex:pid]; - rootInfo.lastRefreshEpoch = epoch; - }); -} - // _lockQueue - Remove PIDs that weren't seen in the current epoch - (void)evictUnseenNodesForRoot:(pid_t)rootPid epoch:(NSUInteger)epoch { iTermTrackedRootInfo *info = _rootsLQ[@(rootPid)]; From 28da00b11413fa2aadca1b94be3aff3b9e6018ea Mon Sep 17 00:00:00 2001 From: chall37 Date: Tue, 3 Feb 2026 11:08:26 -0800 Subject: [PATCH 3/9] Pause new child monitors when parent is paused --- sources/iTermProcessMonitor.m | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sources/iTermProcessMonitor.m b/sources/iTermProcessMonitor.m index 86d70f4156..8abf013c52 100644 --- a/sources/iTermProcessMonitor.m +++ b/sources/iTermProcessMonitor.m @@ -105,6 +105,11 @@ - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo depth:(NSInteger)depth { callback:_callback trackedRootPID:_trackedRootPID]; [child setProcessInfo:childInfo depth:depth + 1]; + // If this monitor is paused, immediately pause the new child so it + // doesn't generate callbacks while the parent is in the background. + if (_isPaused) { + [child pauseMonitoring]; + } [childrenToAdd addObject:child]; }]; [childrenToAdd enumerateObjectsUsingBlock:^(iTermProcessMonitor * _Nonnull child, NSUInteger idx, BOOL * _Nonnull stop) { From f1b40722650d0043b52d3d04b90c1566d14fb424 Mon Sep 17 00:00:00 2001 From: chall37 Date: Tue, 3 Feb 2026 13:53:26 -0800 Subject: [PATCH 4/9] Add unit tests for process cache and process monitor - Add iTermProcessCacheTests covering background refresh cadence, foreground/background priority transitions, and coalescer behavior - Add iTermProcessMonitorTests for pause/resume, hierarchy tracking, and child monitor management - Add +Testing categories to expose internals for test verification - Add iTermProcessCacheTestHelper to bridge ObjC internals to Swift tests - Fix background roots being dropped from dirty set after refresh (re-add alive low-priority roots so they continue periodic updates) --- ModernTests/iTermProcessCacheTests.swift | 439 +++++++++++ ModernTests/iTermProcessMonitorTests.swift | 828 +++++++++++++++++++++ iTerm2.xcodeproj/project.pbxproj | 12 + sources/iTerm2SharedARC-Bridging-Header.h | 2 + sources/iTermProcessCache+Testing.h | 40 + sources/iTermProcessCache+Testing.m | 97 +++ sources/iTermProcessCache.m | 22 +- sources/iTermProcessCacheTestHelper.h | 56 ++ sources/iTermProcessCacheTestHelper.m | 84 +++ sources/iTermProcessMonitor+Testing.h | 22 + sources/iTermProcessMonitor+Testing.m | 28 + 11 files changed, 1626 insertions(+), 4 deletions(-) create mode 100644 ModernTests/iTermProcessCacheTests.swift create mode 100644 ModernTests/iTermProcessMonitorTests.swift create mode 100644 sources/iTermProcessCache+Testing.h create mode 100644 sources/iTermProcessCache+Testing.m create mode 100644 sources/iTermProcessCacheTestHelper.h create mode 100644 sources/iTermProcessCacheTestHelper.m create mode 100644 sources/iTermProcessMonitor+Testing.h create mode 100644 sources/iTermProcessMonitor+Testing.m diff --git a/ModernTests/iTermProcessCacheTests.swift b/ModernTests/iTermProcessCacheTests.swift new file mode 100644 index 0000000000..081c87b194 --- /dev/null +++ b/ModernTests/iTermProcessCacheTests.swift @@ -0,0 +1,439 @@ +// +// iTermProcessCacheTests.swift +// iTerm2 +// +// Tests for iTermProcessCache, covering: +// - Background refresh cadence (P2 fix: background roots stay in dirty set) +// - Foreground/background priority transitions (P3 fix: setForegroundRootPIDs) +// - Coalescer behavior +// + +import XCTest +@testable import iTerm2SharedARC + +// MARK: - Test Class + +final class iTermProcessCacheTests: XCTestCase { + var cache: Any! + + override func setUp() { + super.setUp() + // Create a fresh cache instance for each test using the helper + cache = iTermProcessCacheTestHelper.createTestCache() as Any + } + + override func tearDown() { + cache = nil + super.tearDown() + } + + // MARK: - Helper Methods + + private var dirtyLowRootsCount: UInt { + return iTermProcessCacheTestHelper.dirtyLowRootsCount(forCache: cache) + } + + private var dirtyHighRootsCount: UInt { + return iTermProcessCacheTestHelper.dirtyHighRootsCount(forCache: cache) + } + + private func isRootHighPriority(_ rootPID: pid_t) -> Bool { + return iTermProcessCacheTestHelper.cache(cache, isRootHighPriority: rootPID) + } + + private func isTrackingRoot(_ rootPID: pid_t) -> Bool { + return iTermProcessCacheTestHelper.cache(cache, isTrackingRoot: rootPID) + } + + private func forceBackgroundRefreshTick() { + iTermProcessCacheTestHelper.forceBackgroundRefreshTick(forCache: cache) + } + + private func registerTestRoot(_ rootPID: pid_t) { + iTermProcessCacheTestHelper.cache(cache, registerTestRoot: rootPID) + } + + private func unregisterTestRoot(_ rootPID: pid_t) { + iTermProcessCacheTestHelper.cache(cache, unregisterTestRoot: rootPID) + } + + private func setForegroundRootPIDs(_ pids: Set) { + iTermProcessCacheTestHelper.cache(cache, setForegroundRootPIDs: pids) + } + + // MARK: - 1. Background Refresh Cadence Tests (P2 Fix) + + func testBackgroundRoot_AddedToDirtyLowSet_WhenDemotedToBackground() { + // Given: A root PID registered as foreground + let rootPID = getpid() + registerTestRoot(rootPID) + XCTAssertTrue(isRootHighPriority(rootPID), "Should start as high priority") + + // When: Demoted to background + setForegroundRootPIDs([]) + + // Then: Should be in dirty low set + XCTAssertFalse(isRootHighPriority(rootPID), "Should be low priority after demotion") + XCTAssertGreaterThan(dirtyLowRootsCount, 0, "Background root should be in dirty low set") + } + + func testBackgroundRoot_RemainsInDirtySetAfterRefresh() { + // This test verifies the P2 fix: background roots should be re-added + // to _dirtyLowRootsLQ after being processed. + + // Given: A root PID registered and moved to background + let rootPID = getpid() + registerTestRoot(rootPID) + setForegroundRootPIDs([]) + + let initialDirtyCount = dirtyLowRootsCount + XCTAssertGreaterThan(initialDirtyCount, 0, "Background root should be in dirty low set") + + // When: Force a background refresh tick + forceBackgroundRefreshTick() + + // Allow async work to complete + let expectation = self.expectation(description: "Background refresh completes") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { + expectation.fulfill() + } + waitForExpectations(timeout: 1.0) + + // Then: The root should still be in the dirty low set (the P2 fix) + // If the process is alive, it gets re-added after refresh + // This verifies the fix prevents permanent staleness + XCTAssertTrue(isTrackingRoot(rootPID), "Root should still be tracked") + } + + func testBackgroundRoot_ReenqueuedInDirtyLowSet_AfterRefresh() { + // This test specifically verifies that dirtyLowRootsCount remains > 0 + // after a background refresh tick - catching the "one refresh then stale forever" bug. + + // Given: A root PID registered and moved to background + let rootPID = getpid() + registerTestRoot(rootPID) + setForegroundRootPIDs([]) + + // Verify initial state + let initialDirtyCount = dirtyLowRootsCount + XCTAssertGreaterThanOrEqual(initialDirtyCount, 1, "Background root should be in dirty low set") + + // When: Force a background refresh tick + forceBackgroundRefreshTick() + + // Allow async work to complete + let expectation = self.expectation(description: "Background refresh completes") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) { + expectation.fulfill() + } + waitForExpectations(timeout: 1.0) + + // Then: dirtyLowRootsCount should still be >= 1 (root re-added after processing) + // This is the key assertion that catches the regression + XCTAssertGreaterThanOrEqual(dirtyLowRootsCount, 1, + "Background root should be RE-ADDED to dirty low set after refresh (P2 fix)") + XCTAssertTrue(isTrackingRoot(rootPID), "Root should still be tracked") + } + + func testBackgroundRoots_ProcessedAcrossMultipleTicks() { + // Verify background roots continue refreshing across multiple cadence ticks + + // Given: A root registered and moved to background + let root1 = getpid() + registerTestRoot(root1) + setForegroundRootPIDs([]) + + // When: Multiple background refresh ticks + for i in 0..<5 { + forceBackgroundRefreshTick() + + let expectation = self.expectation(description: "Tick \(i) completes") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + expectation.fulfill() + } + waitForExpectations(timeout: 1.0) + } + + // Then: Should complete without issues, root still tracked + XCTAssertTrue(isTrackingRoot(root1), "Root should still be tracked after multiple ticks") + } + + func testBackgroundRoot_DirtyLowSetSurvivesMultipleTicks() { + // Verify that background roots remain in the dirty low set across multiple + // cadence ticks - ensures repeated cadence refresh does not drain the dirty set. + + // Given: A root registered and moved to background + let rootPID = getpid() + registerTestRoot(rootPID) + setForegroundRootPIDs([]) + + // Initial verification + XCTAssertGreaterThan(dirtyLowRootsCount, 0, "Background root should be in dirty low set initially") + + // When/Then: After each of multiple ticks, dirtyLowRootsCount should remain > 0 + for tickNumber in 1...5 { + forceBackgroundRefreshTick() + + let expectation = self.expectation(description: "Tick \(tickNumber) completes") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.2) { + expectation.fulfill() + } + waitForExpectations(timeout: 1.0) + + // Key assertion: dirty set should NOT be drained + XCTAssertGreaterThan(dirtyLowRootsCount, 0, + "dirtyLowRootsCount should remain > 0 after tick \(tickNumber)") + } + + XCTAssertTrue(isTrackingRoot(rootPID), "Root should still be tracked after multiple ticks") + } + + // MARK: - 2. Foreground Selection Behavior Tests (P3 Fix) + + func testSetForegroundRootPIDs_TransitionsToHighPriority() { + // Given: A root registered as background + let rootPID = getpid() + registerTestRoot(rootPID) + setForegroundRootPIDs([]) + XCTAssertFalse(isRootHighPriority(rootPID), "Should start as low priority") + + // When: Set as foreground + setForegroundRootPIDs([NSNumber(value: rootPID)]) + + // Then: Should be high priority + XCTAssertTrue(isRootHighPriority(rootPID), "Should be high priority after setForegroundRootPIDs") + } + + func testSetForegroundRootPIDs_TransitionsToLowPriority() { + // Given: A root registered as high priority (default) + let rootPID = getpid() + registerTestRoot(rootPID) + XCTAssertTrue(isRootHighPriority(rootPID), "Should start as high priority") + + // When: Remove from foreground set + setForegroundRootPIDs([]) + + // Then: Should be low priority and in dirty low set + XCTAssertFalse(isRootHighPriority(rootPID), "Should be low priority after removal") + XCTAssertGreaterThan(dirtyLowRootsCount, 0, "Should be in dirty low set") + } + + func testSetForegroundRootPIDs_SwitchingBetweenWindows() { + // This simulates the window focus change scenario from P3 + // When user switches windows, the foreground set should update + + // Given: Two roots, root1 is initially foreground + let root1 = getpid() + let root2: pid_t = 99998 + + registerTestRoot(root1) + registerTestRoot(root2) + setForegroundRootPIDs([NSNumber(value: root1)]) + + XCTAssertTrue(isRootHighPriority(root1), "root1 should be foreground") + XCTAssertFalse(isRootHighPriority(root2), "root2 should be background") + + // When: Switch foreground to root2 (simulates switching windows) + setForegroundRootPIDs([NSNumber(value: root2)]) + + // Then: Priorities should swap + XCTAssertFalse(isRootHighPriority(root1), "root1 should now be background") + XCTAssertTrue(isRootHighPriority(root2), "root2 should now be foreground") + } + + func testSetForegroundRootPIDs_MultipleRootsInSameWindow() { + // A window with multiple tabs/sessions should have all their roots as foreground + + // Given: Multiple roots + let root1 = getpid() + let root2: pid_t = 99997 + let root3: pid_t = 99996 + + registerTestRoot(root1) + registerTestRoot(root2) + registerTestRoot(root3) + + // When: Set multiple roots as foreground (simulates window with multiple sessions) + setForegroundRootPIDs([NSNumber(value: root1), NSNumber(value: root2)]) + + // Then: root1 and root2 should be high priority, root3 should be low + XCTAssertTrue(isRootHighPriority(root1), "root1 should be high priority") + XCTAssertTrue(isRootHighPriority(root2), "root2 should be high priority") + XCTAssertFalse(isRootHighPriority(root3), "root3 should be low priority") + } + + func testSetForegroundRootPIDs_Idempotent() { + // Setting the same foreground roots multiple times should be safe + + let rootPID = getpid() + registerTestRoot(rootPID) + setForegroundRootPIDs([NSNumber(value: rootPID)]) + + // When: Set the same foreground roots multiple times + for _ in 0..<5 { + setForegroundRootPIDs([NSNumber(value: rootPID)]) + } + + // Then: Should still be foreground + XCTAssertTrue(isRootHighPriority(rootPID), "Should remain foreground") + } + + // MARK: - 3. Coalescer Behavior Tests + + func testHighPriorityRoot_NotInLowDirtySet() { + // High priority (foreground) roots should use the coalescer, not cadence + + // Given: A root registered as foreground + let rootPID = getpid() + registerTestRoot(rootPID) + setForegroundRootPIDs([NSNumber(value: rootPID)]) + + // Then: Should NOT be in dirty low set (foreground uses coalescer) + // The dirty low set is only for background roots + XCTAssertTrue(isRootHighPriority(rootPID), "Should be high priority") + // After being set as foreground, it may be in dirty high set instead + } + + func testForegroundRoot_NotInDirtyLowSet_AfterMonitorEvent() { + // Foreground roots should go to the high-priority dirty set (coalescer), + // NOT the low-priority dirty set (cadence timer). + + // Given: A root registered and explicitly set as foreground + let rootPID = getpid() + registerTestRoot(rootPID) + setForegroundRootPIDs([NSNumber(value: rootPID)]) + + // Allow registration to complete + let setupExpectation = self.expectation(description: "Setup completes") + DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) { + setupExpectation.fulfill() + } + waitForExpectations(timeout: 1.0) + + // Then: dirtyLowRootsCount should be 0 (foreground roots use coalescer path) + XCTAssertTrue(isRootHighPriority(rootPID), "Root should be high priority") + XCTAssertEqual(dirtyLowRootsCount, 0, + "Foreground root should NOT be in dirty low set - it uses coalescer instead") + } + + func testLowPriorityRoot_InLowDirtySet() { + // Low priority (background) roots should be in the cadence-driven dirty set + + // Given: A root demoted to background + let rootPID = getpid() + registerTestRoot(rootPID) + setForegroundRootPIDs([]) + + // Then: Should be in dirty low set + XCTAssertFalse(isRootHighPriority(rootPID), "Should be low priority") + XCTAssertGreaterThan(dirtyLowRootsCount, 0, "Should be in dirty low set") + } + + // MARK: - 4. Root Registration/Unregistration Tests + + func testRegisterAndUnregisterRoot() { + let rootPID = getpid() + + // When: Register + registerTestRoot(rootPID) + XCTAssertTrue(isTrackingRoot(rootPID), "Should be tracked after registration") + + // When: Unregister + unregisterTestRoot(rootPID) + XCTAssertFalse(isTrackingRoot(rootPID), "Should not be tracked after unregistration") + } + + func testUnregisterRoot_ClearsFromDirtySets() { + // Given: A root in background (in dirty low set) + let rootPID = getpid() + registerTestRoot(rootPID) + setForegroundRootPIDs([]) + + let dirtyCountBefore = dirtyLowRootsCount + XCTAssertGreaterThan(dirtyCountBefore, 0, "Should be in dirty low set") + + // When: Unregister + unregisterTestRoot(rootPID) + + // Then: Should be removed from dirty sets + let dirtyCountAfter = dirtyLowRootsCount + XCTAssertLessThan(dirtyCountAfter, dirtyCountBefore, "Should be removed from dirty set") + } + + // MARK: - 5. Concurrent Access Tests + + func testConcurrentForegroundRootPIDUpdates() { + // Verify thread safety of setForegroundRootPIDs + + let root1 = getpid() + let root2: pid_t = 99995 + let root3: pid_t = 99994 + + registerTestRoot(root1) + registerTestRoot(root2) + registerTestRoot(root3) + + let expectation = self.expectation(description: "Concurrent updates complete") + expectation.expectedFulfillmentCount = 10 + + // When: Concurrent updates from multiple queues + for i in 0..<10 { + DispatchQueue.global().async { + let foregroundSet: Set + switch i % 3 { + case 0: + foregroundSet = [NSNumber(value: root1)] + case 1: + foregroundSet = [NSNumber(value: root2)] + default: + foregroundSet = [NSNumber(value: root3)] + } + self.setForegroundRootPIDs(foregroundSet) + expectation.fulfill() + } + } + + waitForExpectations(timeout: 5.0) + + // Then: All roots should still be tracked (no corruption) + XCTAssertTrue(isTrackingRoot(root1), "root1 should still be tracked") + XCTAssertTrue(isTrackingRoot(root2), "root2 should still be tracked") + XCTAssertTrue(isTrackingRoot(root3), "root3 should still be tracked") + } + + // MARK: - 6. Edge Cases + + func testSetForegroundRootPIDs_EmptySet() { + let rootPID = getpid() + registerTestRoot(rootPID) + setForegroundRootPIDs([NSNumber(value: rootPID)]) + + // When: Set empty foreground set + setForegroundRootPIDs([]) + + // Then: All roots should be background + XCTAssertFalse(isRootHighPriority(rootPID), "Should be background with empty foreground set") + } + + func testSetForegroundRootPIDs_UnregisteredPID() { + // Setting foreground with an unregistered PID should be safe + let unregisteredPID: pid_t = 88888 + + // When: Set unregistered PID as foreground + setForegroundRootPIDs([NSNumber(value: unregisteredPID)]) + + // Then: Should not crash, unregistered PID should not become tracked + XCTAssertFalse(isTrackingRoot(unregisteredPID), "Unregistered PID should not be tracked") + } + + func testBackgroundRefreshTick_EmptyDirtySet() { + // Calling backgroundRefreshTick with no dirty roots should be safe + XCTAssertEqual(dirtyLowRootsCount, 0, "Should start with empty dirty set") + + // When: Force background refresh + forceBackgroundRefreshTick() + + // Then: Should not crash + XCTAssertEqual(dirtyLowRootsCount, 0, "Should still be empty") + } +} diff --git a/ModernTests/iTermProcessMonitorTests.swift b/ModernTests/iTermProcessMonitorTests.swift new file mode 100644 index 0000000000..70be6466e6 --- /dev/null +++ b/ModernTests/iTermProcessMonitorTests.swift @@ -0,0 +1,828 @@ +// +// iTermProcessMonitorTests.swift +// iTerm2 +// +// Created by George Nachman on 2/3/26. +// + +import XCTest +@testable import iTerm2SharedARC + +// MARK: - Mock Objects + +class MockProcessDataSource: NSObject, ProcessDataSource { + var processNames: [pid_t: String] = [:] + var foregroundPIDs: Set = [] + var commandLines: [pid_t: [String]] = [:] + var startTimes: [pid_t: Date] = [:] + + func nameOfProcess(withPid thePid: pid_t, isForeground: UnsafeMutablePointer) -> String? { + isForeground.pointee = ObjCBool(foregroundPIDs.contains(thePid)) + return processNames[thePid] + } + + func commandLineArguments(forProcess pid: pid_t, execName: AutoreleasingUnsafeMutablePointer?) -> [String]? { + if let argv = commandLines[pid], let first = argv.first { + execName?.pointee = first as NSString + } + return commandLines[pid] + } + + func startTime(forProcess pid: pid_t) -> Date? { + return startTimes[pid] + } +} + +// MARK: - Test Helpers + +/// Helper to create a process collection with mock data +private func makeProcessCollection(dataSource: MockProcessDataSource, + processes: [(pid: pid_t, ppid: pid_t)]) -> ProcessCollection { + let collection = ProcessCollection(dataSource: dataSource) + for (pid, ppid) in processes { + collection.addProcess(withProcessID: pid, parentProcessID: ppid) + } + collection.commit() + return collection +} + +// MARK: - Test Class + +final class iTermProcessMonitorTests: XCTestCase { + var dataSource: MockProcessDataSource! + var testQueue: DispatchQueue! + var callbackEvents: [(monitor: iTermProcessMonitor, flags: UInt)]! + var callbackExpectation: XCTestExpectation? + + override func setUp() { + super.setUp() + dataSource = MockProcessDataSource() + testQueue = DispatchQueue(label: "com.iterm2.test.processmonitor") + callbackEvents = [] + } + + override func tearDown() { + dataSource = nil + testQueue = nil + callbackEvents = nil + callbackExpectation = nil + super.tearDown() + } + + private func makeCallback() -> (iTermProcessMonitor, UInt) -> Void { + return { [weak self] monitor, flags in + self?.callbackEvents.append((monitor: monitor, flags: flags)) + self?.callbackExpectation?.fulfill() + } + } + + // MARK: - Monitor Pause State Helpers + + private func isMonitorPaused(_ monitor: Any) -> Bool { + return iTermProcessCacheTestHelper.monitorIsPaused(monitor) + } + + private func childMonitors(for monitor: Any) -> [Any] { + return iTermProcessCacheTestHelper.childMonitors(forMonitor: monitor) ?? [] + } + + // MARK: - 1. Initialization Tests + + func testInit_SetsQueueCallbackAndTrackedRootPID() { + // Given + let rootPID: pid_t = 12345 + let callback = makeCallback() + + // When + let monitor = iTermProcessMonitor(queue: testQueue, callback: callback, trackedRootPID: rootPID) + + // Then + XCTAssertEqual(monitor.trackedRootPID, rootPID) + XCTAssertNotNil(monitor.queue) + XCTAssertNotNil(monitor.callback) + XCTAssertNil(monitor.processInfo) + XCTAssertNil(monitor.parent) + } + + func testInit_LegacyInitializer_TrackedRootPIDIsZero() { + // Given + let callback = makeCallback() + + // When + let monitor = iTermProcessMonitor(queue: testQueue, callback: callback) + + // Then + XCTAssertEqual(monitor.trackedRootPID, 0) + } + + // MARK: - 2. setProcessInfo Tests + + func testSetProcessInfo_WithValidPID_ReturnsYES() { + // Given + let pid = getpid() + let ppid = getppid() + dataSource.processNames[pid] = "xctest" + dataSource.processNames[ppid] = "launchd" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, ppid)]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) + + // When + let changed = monitor.setProcessInfo(processInfo) + + // Then + XCTAssertTrue(changed) + XCTAssertEqual(monitor.processInfo?.processID, pid) + } + + func testSetProcessInfo_SameInstance_ReturnsNO() { + // Given + let pid = getpid() + dataSource.processNames[pid] = "xctest" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) + _ = monitor.setProcessInfo(processInfo) + + // When + let changed = monitor.setProcessInfo(processInfo) + + // Then + XCTAssertFalse(changed) + } + + func testSetProcessInfo_Nil_InvalidatesMonitor() { + // Given + let pid = getpid() + dataSource.processNames[pid] = "xctest" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) + _ = monitor.setProcessInfo(processInfo) + XCTAssertNotNil(monitor.processInfo) + + // When + let changed = monitor.setProcessInfo(nil as iTermProcessInfo?) + + // Then + XCTAssertTrue(changed) + XCTAssertNil(monitor.processInfo) + } + + // MARK: - 3. Child Monitor Management Tests + + func testSetProcessInfo_WithChildren_CreatesChildMonitors() { + // Given: A process tree with parent and two children + let parentPID = getpid() + let child1PID: pid_t = 99991 + let child2PID: pid_t = 99992 + + dataSource.processNames[parentPID] = "xctest" + dataSource.processNames[child1PID] = "child1" + dataSource.processNames[child2PID] = "child2" + + // Build collection with parent-child relationships + let collection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (child1PID, parentPID), + (child2PID, parentPID) + ]) + + guard let parentInfo = collection.info(forProcessID: parentPID) else { + XCTFail("Failed to create parent process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) + + // When + let changed = monitor.setProcessInfo(parentInfo) + + // Then + XCTAssertTrue(changed) + // Children should exist (verified via the process info tree) + XCTAssertEqual(parentInfo.children.count, 2) + } + + func testSetProcessInfo_TrackedRootPID_PropagatedToChildren() { + // Given: A process tree + let rootPID: pid_t = 12345 + let parentPID = getpid() + let childPID: pid_t = 99991 + + dataSource.processNames[parentPID] = "xctest" + dataSource.processNames[childPID] = "child" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (childPID, parentPID) + ]) + + guard let parentInfo = collection.info(forProcessID: parentPID) else { + XCTFail("Failed to create parent process info") + return + } + + // Monitor with specific trackedRootPID + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: rootPID) + + // When + _ = monitor.setProcessInfo(parentInfo) + + // Then + XCTAssertEqual(monitor.trackedRootPID, rootPID) + // The trackedRootPID is propagated to children (tested via callback behavior) + } + + // MARK: - 4. Pause/Resume Tests + + func testPauseMonitoring_Idempotent() { + // Given: A monitor with process info + let pid = getpid() + dataSource.processNames[pid] = "xctest" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) + _ = monitor.setProcessInfo(processInfo) + + // When: Pause multiple times + testQueue.sync { + monitor.pauseMonitoring() + monitor.pauseMonitoring() // Should not crash or cause issues + monitor.pauseMonitoring() + } + + // Then: No crash, can still resume + testQueue.sync { + monitor.resumeMonitoring() + } + } + + func testResumeMonitoring_Idempotent() { + // Given: A monitor with process info (not paused) + let pid = getpid() + dataSource.processNames[pid] = "xctest" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) + _ = monitor.setProcessInfo(processInfo) + + // When: Resume multiple times without pausing first + testQueue.sync { + monitor.resumeMonitoring() // Should not crash + monitor.resumeMonitoring() + } + + // Then: No crash + } + + func testPauseMonitoring_WhenNoSource_DoesNotCrash() { + // Given: A monitor without process info (no dispatch source) + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: 0) + + // When/Then: Should not crash + testQueue.sync { + monitor.pauseMonitoring() + } + } + + func testResumeMonitoring_WhenNoSource_DoesNotCrash() { + // Given: A monitor without process info (no dispatch source) + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: 0) + + // When/Then: Should not crash + testQueue.sync { + monitor.resumeMonitoring() + } + } + + // MARK: - 5. KEY FIX TESTS: New Children Paused When Parent Is Paused + + func testPausedMonitor_NewChildIsPaused() { + // This test verifies that when a paused parent creates a new child via + // setProcessInfo, the child monitor's dispatch source is also paused. + // This catches the child monitor activation while parent is paused bug. + + // Given: A monitor with initial process info (no children) + let parentPID = getpid() + dataSource.processNames[parentPID] = "xctest" + + let initialCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()) + ]) + + guard let initialInfo = initialCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create initial process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) + _ = monitor.setProcessInfo(initialInfo) + + // Pause the parent monitor + testQueue.sync { + monitor.pauseMonitoring() + } + + // Verify parent is paused + var parentPaused = false + testQueue.sync { + parentPaused = isMonitorPaused(monitor) + } + XCTAssertTrue(parentPaused, "Parent monitor should be paused") + + // When: Update process info to add a child while parent is paused + let childPID: pid_t = 99991 + dataSource.processNames[childPID] = "child" + + let updatedCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (childPID, parentPID) + ]) + + guard let updatedInfo = updatedCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create updated process info") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(updatedInfo) + } + + // Then: The newly created child monitor should also be paused + testQueue.sync { + let children = childMonitors(for: monitor) + XCTAssertEqual(children.count, 1, "Should have one child monitor") + + if let childMonitor = children.first { + XCTAssertTrue(isMonitorPaused(childMonitor), + "NEW child monitor should be PAUSED when created while parent is paused") + } + } + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } + + func testPausedMonitor_RemainsPausedAfterProcessInfoUpdate() { + // Verifies that updating process info on a paused monitor does not + // implicitly resume the parent or any children. + + // Given: A monitor with one child, then paused + let parentPID = getpid() + let existingChildPID: pid_t = 99991 + + dataSource.processNames[parentPID] = "xctest" + dataSource.processNames[existingChildPID] = "existingChild" + + let initialCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (existingChildPID, parentPID) + ]) + + guard let initialInfo = initialCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create initial process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) + _ = monitor.setProcessInfo(initialInfo) + + // Pause the monitor tree + testQueue.sync { + monitor.pauseMonitoring() + } + + // Verify everything is paused + testQueue.sync { + XCTAssertTrue(isMonitorPaused(monitor), "Parent should be paused") + let children = childMonitors(for: monitor) + for child in children { + XCTAssertTrue(isMonitorPaused(child), "Existing child should be paused") + } + } + + // When: Update process info with a new child + let newChildPID: pid_t = 99992 + dataSource.processNames[newChildPID] = "newChild" + + let updatedCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (existingChildPID, parentPID), + (newChildPID, parentPID) + ]) + + guard let updatedInfo = updatedCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create updated process info") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(updatedInfo) + } + + // Then: Parent should STILL be paused, and ALL children should be paused + testQueue.sync { + XCTAssertTrue(isMonitorPaused(monitor), + "Parent should REMAIN paused after processInfo update") + + let children = childMonitors(for: monitor) + XCTAssertEqual(children.count, 2, "Should have two children") + + for (index, child) in children.enumerated() { + XCTAssertTrue(isMonitorPaused(child), + "Child \(index) should be paused after processInfo update") + } + } + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } + + func testSetProcessInfo_ParentPaused_NewChildMonitorsPauseAndResumeCorrectly() { + // This test verifies the fix: when a parent is paused and setProcessInfo + // creates new children, those children should also be paused. We verify + // this by checking that resume works correctly (if children weren't paused, + // calling resume on them would be undefined behavior for dispatch sources). + + // Given: A monitor with initial process info (no children) + let parentPID = getpid() + dataSource.processNames[parentPID] = "xctest" + + let initialCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()) + ]) + + guard let initialInfo = initialCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create initial process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) + _ = monitor.setProcessInfo(initialInfo) + + // Pause the parent monitor + testQueue.sync { + monitor.pauseMonitoring() + } + + // When: Update process info to add children while parent is paused + let childPID: pid_t = 99991 + dataSource.processNames[childPID] = "child" + + let updatedCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (childPID, parentPID) + ]) + + guard let updatedInfo = updatedCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create updated process info") + return + } + + testQueue.sync { + // This creates a new child monitor. With the fix, it should be paused. + _ = monitor.setProcessInfo(updatedInfo) + } + + // Then: Resume should work without issues (verifies children were properly paused) + testQueue.sync { + // If new children weren't paused, this could cause undefined behavior + // when resumeMonitoring tries to resume the parent and its children + monitor.resumeMonitoring() + } + + // Additional verification: can pause and resume again + testQueue.sync { + monitor.pauseMonitoring() + monitor.resumeMonitoring() + } + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } + + func testSetProcessInfo_ParentPaused_MultipleNewChildren_AllPauseCorrectly() { + // Given: A monitor with initial process info (no children) + let parentPID = getpid() + dataSource.processNames[parentPID] = "xctest" + + let initialCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()) + ]) + + guard let initialInfo = initialCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create initial process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) + _ = monitor.setProcessInfo(initialInfo) + + // Pause the parent + testQueue.sync { + monitor.pauseMonitoring() + } + + // When: Add multiple children while paused + let child1PID: pid_t = 99991 + let child2PID: pid_t = 99992 + let child3PID: pid_t = 99993 + + dataSource.processNames[child1PID] = "child1" + dataSource.processNames[child2PID] = "child2" + dataSource.processNames[child3PID] = "child3" + + let updatedCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (child1PID, parentPID), + (child2PID, parentPID), + (child3PID, parentPID) + ]) + + guard let updatedInfo = updatedCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create updated process info") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(updatedInfo) + } + + // Then: Resume should handle all children correctly + testQueue.sync { + monitor.resumeMonitoring() + } + + // Verify multiple pause/resume cycles work + for _ in 0..<3 { + testQueue.sync { + monitor.pauseMonitoring() + monitor.resumeMonitoring() + } + } + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } + + func testSetProcessInfo_ParentNotPaused_ChildrenNotAffected() { + // Given: A monitor that is NOT paused + let parentPID = getpid() + dataSource.processNames[parentPID] = "xctest" + + let initialCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()) + ]) + + guard let initialInfo = initialCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create initial process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) + _ = monitor.setProcessInfo(initialInfo) + + // Parent is NOT paused + + // When: Add children + let childPID: pid_t = 99991 + dataSource.processNames[childPID] = "child" + + let updatedCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (childPID, parentPID) + ]) + + guard let updatedInfo = updatedCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create updated process info") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(updatedInfo) + } + + // Then: Can pause and resume the whole tree + testQueue.sync { + monitor.pauseMonitoring() + monitor.resumeMonitoring() + } + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } + + // MARK: - 6. Invalidation Tests + + func testInvalidate_ClearsProcessInfo() { + // Given: A monitor with process info + let pid = getpid() + dataSource.processNames[pid] = "xctest" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) + _ = monitor.setProcessInfo(processInfo) + XCTAssertNotNil(monitor.processInfo) + + // When + testQueue.sync { + monitor.invalidate() + } + + // Then + XCTAssertNil(monitor.processInfo) + } + + func testInvalidate_WhenPaused_DoesNotCrash() { + // Given: A paused monitor + let pid = getpid() + dataSource.processNames[pid] = "xctest" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) + _ = monitor.setProcessInfo(processInfo) + + testQueue.sync { + monitor.pauseMonitoring() + } + + // When/Then: Should not crash (must resume before cancel per dispatch API) + testQueue.sync { + monitor.invalidate() + } + + XCTAssertNil(monitor.processInfo) + } + + func testInvalidate_Idempotent() { + // Given: A monitor + let pid = getpid() + dataSource.processNames[pid] = "xctest" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) + _ = monitor.setProcessInfo(processInfo) + + // When: Invalidate multiple times + testQueue.sync { + monitor.invalidate() + monitor.invalidate() // Should not crash + monitor.invalidate() + } + + // Then: No crash + XCTAssertNil(monitor.processInfo) + } + + // MARK: - 7. Recursive Pause/Resume with Children Tests + + func testPauseAndResume_WithExistingChildren_WorksCorrectly() { + // Given: A monitor with existing children + let parentPID = getpid() + let childPID: pid_t = 99991 + + dataSource.processNames[parentPID] = "xctest" + dataSource.processNames[childPID] = "child" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (childPID, parentPID) + ]) + + guard let parentInfo = collection.info(forProcessID: parentPID) else { + XCTFail("Failed to create parent process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) + _ = monitor.setProcessInfo(parentInfo) + + // When: Pause and resume multiple times + for _ in 0..<5 { + testQueue.sync { + monitor.pauseMonitoring() + } + testQueue.sync { + monitor.resumeMonitoring() + } + } + + // Then: Should complete without issues + testQueue.sync { + monitor.invalidate() + } + } + + func testPauseAddChildrenResume_ChildrenProperlyManaged() { + // Given: A monitor with one child + let parentPID = getpid() + let existingChildPID: pid_t = 99991 + + dataSource.processNames[parentPID] = "xctest" + dataSource.processNames[existingChildPID] = "existingChild" + + let initialCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (existingChildPID, parentPID) + ]) + + guard let initialInfo = initialCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create initial process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) + _ = monitor.setProcessInfo(initialInfo) + + // Pause + testQueue.sync { + monitor.pauseMonitoring() + } + + // When: Add a new child while paused + let newChildPID: pid_t = 99992 + dataSource.processNames[newChildPID] = "newChild" + + let updatedCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (existingChildPID, parentPID), + (newChildPID, parentPID) + ]) + + guard let updatedInfo = updatedCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create updated process info") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(updatedInfo) + } + + // Then: Resume should properly handle both existing and new children + testQueue.sync { + monitor.resumeMonitoring() + } + + // Verify we can do another pause/resume cycle + testQueue.sync { + monitor.pauseMonitoring() + monitor.resumeMonitoring() + } + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } +} diff --git a/iTerm2.xcodeproj/project.pbxproj b/iTerm2.xcodeproj/project.pbxproj index 5b06a62524..587e68f53a 100644 --- a/iTerm2.xcodeproj/project.pbxproj +++ b/iTerm2.xcodeproj/project.pbxproj @@ -815,6 +815,8 @@ 537BFDC520FFA5A40098C91F /* iTermStatusBarJobComponent.h in Headers */ = {isa = PBXBuildFile; fileRef = 537BFDC320FFA5A40098C91F /* iTermStatusBarJobComponent.h */; }; 537BFDC920FFB2590098C91F /* iTermProcessCache.h in Headers */ = {isa = PBXBuildFile; fileRef = 537BFDC720FFB2590098C91F /* iTermProcessCache.h */; }; 537BFDCA20FFB2590098C91F /* iTermProcessCache.m in Sources */ = {isa = PBXBuildFile; fileRef = 537BFDC820FFB2590098C91F /* iTermProcessCache.m */; }; + BAB0C1C2780DA546AFE73B80 /* iTermProcessCache+Testing.m in Sources */ = {isa = PBXBuildFile; fileRef = 42BC3DE2E88700AF9526F074 /* iTermProcessCache+Testing.m */; }; + 2A1F4551EB4181AC933F6915 /* iTermProcessCacheTestHelper.m in Sources */ = {isa = PBXBuildFile; fileRef = 08829F633C36167ABC85693D /* iTermProcessCacheTestHelper.m */; }; 537BFDD12101AD9F0098C91F /* iTermCPUUtilization.h in Headers */ = {isa = PBXBuildFile; fileRef = 537BFDCF2101AD9F0098C91F /* iTermCPUUtilization.h */; }; 537BFDD22101AD9F0098C91F /* iTermCPUUtilization.m in Sources */ = {isa = PBXBuildFile; fileRef = 537BFDD02101AD9F0098C91F /* iTermCPUUtilization.m */; }; 537BFDD52101B2500098C91F /* iTermStatusBarCPUUtilizationComponent.h in Headers */ = {isa = PBXBuildFile; fileRef = 537BFDD32101B2500098C91F /* iTermStatusBarCPUUtilizationComponent.h */; }; @@ -6076,6 +6078,10 @@ 537BFDC420FFA5A40098C91F /* iTermStatusBarJobComponent.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = iTermStatusBarJobComponent.m; sourceTree = ""; }; 537BFDC720FFB2590098C91F /* iTermProcessCache.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = iTermProcessCache.h; sourceTree = ""; }; 537BFDC820FFB2590098C91F /* iTermProcessCache.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = iTermProcessCache.m; sourceTree = ""; }; + C47650BBAFA6A247F2489C37 /* iTermProcessCache+Testing.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "iTermProcessCache+Testing.h"; sourceTree = ""; }; + 42BC3DE2E88700AF9526F074 /* iTermProcessCache+Testing.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "iTermProcessCache+Testing.m"; sourceTree = ""; }; + 0FB571F65DACC53FDDE3E286 /* iTermProcessCacheTestHelper.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = iTermProcessCacheTestHelper.h; sourceTree = ""; }; + 08829F633C36167ABC85693D /* iTermProcessCacheTestHelper.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = iTermProcessCacheTestHelper.m; sourceTree = ""; }; 537BFDCF2101AD9F0098C91F /* iTermCPUUtilization.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = iTermCPUUtilization.h; sourceTree = ""; }; 537BFDD02101AD9F0098C91F /* iTermCPUUtilization.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = iTermCPUUtilization.m; sourceTree = ""; }; 537BFDD32101B2500098C91F /* iTermStatusBarCPUUtilizationComponent.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = iTermStatusBarCPUUtilizationComponent.h; sourceTree = ""; }; @@ -14203,6 +14209,10 @@ children = ( 537BFDC720FFB2590098C91F /* iTermProcessCache.h */, 537BFDC820FFB2590098C91F /* iTermProcessCache.m */, + C47650BBAFA6A247F2489C37 /* iTermProcessCache+Testing.h */, + 42BC3DE2E88700AF9526F074 /* iTermProcessCache+Testing.m */, + 0FB571F65DACC53FDDE3E286 /* iTermProcessCacheTestHelper.h */, + 08829F633C36167ABC85693D /* iTermProcessCacheTestHelper.m */, 53AFFC8A1DD2A03800E6CEC6 /* iTermLSOF.h */, 53AFFC8B1DD2A03800E6CEC6 /* iTermLSOF.m */, A6EEAEDD283712070039E850 /* ProcessDataSource.swift */, @@ -19638,6 +19648,8 @@ A6D3205529C29EE30088BBE1 /* NerdFontInstaller.swift in Sources */, 5308BF8A22828268004BECAC /* iTermStatusBarBatteryComponent.m in Sources */, 537BFDCA20FFB2590098C91F /* iTermProcessCache.m in Sources */, + BAB0C1C2780DA546AFE73B80 /* iTermProcessCache+Testing.m in Sources */, + 2A1F4551EB4181AC933F6915 /* iTermProcessCacheTestHelper.m in Sources */, A6F9EF3820850C41005530F7 /* iTermCompetentTableRowView.m in Sources */, A6D10E3427F7C2BC0026DB56 /* NSRange+MultiCursor.swift in Sources */, A65B1BD32D1E31CB00F5A740 /* iTermMetalView.swift in Sources */, diff --git a/sources/iTerm2SharedARC-Bridging-Header.h b/sources/iTerm2SharedARC-Bridging-Header.h index fa7fc26c4b..fd442c5232 100644 --- a/sources/iTerm2SharedARC-Bridging-Header.h +++ b/sources/iTerm2SharedARC-Bridging-Header.h @@ -66,6 +66,8 @@ #import "iTermPreciseTimer.h" #import "iTermPreferences.h" #import "iTermProcess.h" +#import "iTermProcessCacheTestHelper.h" +#import "iTermProcessMonitor.h" #import "iTermProfilePreferencesBaseViewController.h" #import "iTermPromise.h" #import "iTermRemotePreferences.h" diff --git a/sources/iTermProcessCache+Testing.h b/sources/iTermProcessCache+Testing.h new file mode 100644 index 0000000000..2c0fd36211 --- /dev/null +++ b/sources/iTermProcessCache+Testing.h @@ -0,0 +1,40 @@ +// +// iTermProcessCache+Testing.h +// iTerm2SharedARC +// +// Testing-only interface for iTermProcessCache. +// + +#import "iTermProcessCache.h" + +NS_ASSUME_NONNULL_BEGIN + +@interface iTermProcessCache (Testing) + +/// Create a non-singleton instance for testing. +- (instancetype)initForTesting; + +/// Number of roots in the dirty low-priority (background) set. +@property (nonatomic, readonly) NSUInteger dirtyLowRootsCount; + +/// Number of roots in the dirty high-priority (foreground) set. +@property (nonatomic, readonly) NSUInteger dirtyHighRootsCount; + +/// Whether a specific root PID is currently marked as high priority (foreground). +- (BOOL)isRootHighPriority:(pid_t)rootPID; + +/// Whether a specific root PID exists in the tracking system. +- (BOOL)isTrackingRoot:(pid_t)rootPID; + +/// Force a background refresh tick (normally called by timer). +- (void)forceBackgroundRefreshTick; + +/// Register a root PID for tracking (testing only). +- (void)registerTestRoot:(pid_t)rootPID; + +/// Unregister a root PID (testing only). +- (void)unregisterTestRoot:(pid_t)rootPID; + +@end + +NS_ASSUME_NONNULL_END diff --git a/sources/iTermProcessCache+Testing.m b/sources/iTermProcessCache+Testing.m new file mode 100644 index 0000000000..75e3b7e3cd --- /dev/null +++ b/sources/iTermProcessCache+Testing.m @@ -0,0 +1,97 @@ +// +// iTermProcessCache+Testing.m +// iTerm2SharedARC +// +// Testing-only implementation for iTermProcessCache. +// + +#import "iTermProcessCache+Testing.h" +#import "iTermProcessMonitor.h" + +// Forward declare private ivars we need to access +@interface iTermProcessCache () { + @public + dispatch_queue_t _lockQueue; + dispatch_queue_t _workQueue; + NSMutableDictionary *_rootsLQ; + NSMutableIndexSet *_dirtyHighRootsLQ; + NSMutableIndexSet *_dirtyLowRootsLQ; +} +- (void)backgroundRefreshTick; +@end + +// Forward declare iTermTrackedRootInfo for access +@interface iTermTrackedRootInfo : NSObject +@property (nonatomic) BOOL isHighPriority; +@property (nonatomic, strong, nullable) iTermProcessMonitor *monitor; +@property (nonatomic, strong) NSMutableIndexSet *cachedDescendants; +@property (nonatomic) NSUInteger lastRefreshEpoch; +@property (nonatomic) BOOL isDirty; +@end + +@implementation iTermProcessCache (Testing) + +- (instancetype)initForTesting { + return [self init]; +} + +- (NSUInteger)dirtyLowRootsCount { + __block NSUInteger count = 0; + dispatch_sync(_lockQueue, ^{ + count = self->_dirtyLowRootsLQ.count; + }); + return count; +} + +- (NSUInteger)dirtyHighRootsCount { + __block NSUInteger count = 0; + dispatch_sync(_lockQueue, ^{ + count = self->_dirtyHighRootsLQ.count; + }); + return count; +} + +- (BOOL)isRootHighPriority:(pid_t)rootPID { + __block BOOL result = NO; + dispatch_sync(_lockQueue, ^{ + iTermTrackedRootInfo *info = self->_rootsLQ[@(rootPID)]; + result = info.isHighPriority; + }); + return result; +} + +- (BOOL)isTrackingRoot:(pid_t)rootPID { + __block BOOL result = NO; + dispatch_sync(_lockQueue, ^{ + result = self->_rootsLQ[@(rootPID)] != nil; + }); + return result; +} + +- (void)forceBackgroundRefreshTick { + [self backgroundRefreshTick]; +} + +- (void)registerTestRoot:(pid_t)rootPID { + dispatch_sync(_lockQueue, ^{ + if (self->_rootsLQ[@(rootPID)] == nil) { + iTermTrackedRootInfo *info = [[iTermTrackedRootInfo alloc] init]; + info.isHighPriority = YES; // Default to foreground + self->_rootsLQ[@(rootPID)] = info; + } + }); +} + +- (void)unregisterTestRoot:(pid_t)rootPID { + dispatch_sync(_lockQueue, ^{ + iTermTrackedRootInfo *info = self->_rootsLQ[@(rootPID)]; + if (info) { + [info.monitor invalidate]; + [self->_rootsLQ removeObjectForKey:@(rootPID)]; + [self->_dirtyHighRootsLQ removeIndex:rootPID]; + [self->_dirtyLowRootsLQ removeIndex:rootPID]; + } + }); +} + +@end diff --git a/sources/iTermProcessCache.m b/sources/iTermProcessCache.m index 40b3f34a23..8539334c9b 100644 --- a/sources/iTermProcessCache.m +++ b/sources/iTermProcessCache.m @@ -809,16 +809,30 @@ - (void)backgroundRefreshTick { } // Update cache with results and remove confirmed dead entries - if (newCache.count > 0 || confirmedDeadRoots.count > 0) { - dispatch_sync(self->_lockQueue, ^{ + NSSet *deadSet = [NSSet setWithArray:confirmedDeadRoots]; + dispatch_sync(self->_lockQueue, ^{ + if (newCache.count > 0 || confirmedDeadRoots.count > 0) { NSMutableDictionary *mutableCache = [self->_cachedDeepestForegroundJobLQ mutableCopy] ?: [NSMutableDictionary dictionary]; [mutableCache addEntriesFromDictionary:newCache]; for (NSNumber *deadPid in confirmedDeadRoots) { [mutableCache removeObjectForKey:deadPid]; } self->_cachedDeepestForegroundJobLQ = [mutableCache copy]; - }); - } + } + + // Re-add ALL alive roots that are still background (low priority) + // so they continue getting periodic updates on the 0.5s cadence. + // This includes roots where refresh returned nil due to stale collection. + for (NSNumber *pidNum in toRefresh) { + if ([deadSet containsObject:pidNum]) { + continue; // Don't re-add confirmed dead roots + } + iTermTrackedRootInfo *info = self->_rootsLQ[pidNum]; + if (info && !info.isHighPriority) { + [self->_dirtyLowRootsLQ addIndex:pidNum.unsignedIntegerValue]; + } + } + }); }); } diff --git a/sources/iTermProcessCacheTestHelper.h b/sources/iTermProcessCacheTestHelper.h new file mode 100644 index 0000000000..f69b415615 --- /dev/null +++ b/sources/iTermProcessCacheTestHelper.h @@ -0,0 +1,56 @@ +// +// iTermProcessCacheTestHelper.h +// iTerm2SharedARC +// +// A Swift-compatible wrapper to expose iTermProcessCache testing methods. +// This avoids the circular header dependency between iTermProcessCache.h and Swift. +// + +#import + +NS_ASSUME_NONNULL_BEGIN + +/// Helper class to access iTermProcessCache testing methods from Swift. +/// This wrapper avoids the circular dependency caused by importing iTermProcessCache.h +/// directly into the Swift bridging header. +@interface iTermProcessCacheTestHelper : NSObject + +/// Create a new iTermProcessCache instance for testing (not the singleton). ++ (id)createTestCache; + +/// Number of roots in the dirty low-priority (background) set. ++ (NSUInteger)dirtyLowRootsCountForCache:(id)cache; + +/// Number of roots in the dirty high-priority (foreground) set. ++ (NSUInteger)dirtyHighRootsCountForCache:(id)cache; + +/// Whether a specific root PID is currently marked as high priority (foreground). ++ (BOOL)cache:(id)cache isRootHighPriority:(pid_t)rootPID; + +/// Whether a specific root PID exists in the tracking system. ++ (BOOL)cache:(id)cache isTrackingRoot:(pid_t)rootPID; + +/// Force a background refresh tick (normally called by timer). ++ (void)forceBackgroundRefreshTickForCache:(id)cache; + +/// Register a root PID for tracking (testing only). ++ (void)cache:(id)cache registerTestRoot:(pid_t)rootPID; + +/// Unregister a root PID (testing only). ++ (void)cache:(id)cache unregisterTestRoot:(pid_t)rootPID; + +/// Set foreground root PIDs. ++ (void)cache:(id)cache setForegroundRootPIDs:(NSSet *)foregroundPIDs; + +/// Get the monitor for a root PID (returns id to avoid header dependency). ++ (nullable id)cache:(id)cache monitorForRoot:(pid_t)rootPID; + +/// Check if a monitor is paused. ++ (BOOL)monitorIsPaused:(id)monitor; + +/// Get child monitors from a monitor. ++ (NSArray *)childMonitorsForMonitor:(id)monitor; + +@end + +NS_ASSUME_NONNULL_END diff --git a/sources/iTermProcessCacheTestHelper.m b/sources/iTermProcessCacheTestHelper.m new file mode 100644 index 0000000000..b29c7910b6 --- /dev/null +++ b/sources/iTermProcessCacheTestHelper.m @@ -0,0 +1,84 @@ +// +// iTermProcessCacheTestHelper.m +// iTerm2SharedARC +// +// A Swift-compatible wrapper to expose iTermProcessCache testing methods. +// + +#import "iTermProcessCacheTestHelper.h" +#import "iTermProcessCache.h" +#import "iTermProcessCache+Testing.h" +#import "iTermProcessMonitor.h" +#import "iTermProcessMonitor+Testing.h" + +// Forward declare iTermTrackedRootInfo for access +@interface iTermTrackedRootInfo : NSObject +@property (nonatomic) BOOL isHighPriority; +@property (nonatomic, strong, nullable) iTermProcessMonitor *monitor; +@end + +// Forward declare private ivars we need to access +@interface iTermProcessCache () { + @public + dispatch_queue_t _lockQueue; + NSMutableDictionary *_rootsLQ; +} +@end + +@implementation iTermProcessCacheTestHelper + ++ (id)createTestCache { + return [[iTermProcessCache alloc] initForTesting]; +} + ++ (NSUInteger)dirtyLowRootsCountForCache:(id)cache { + return [(iTermProcessCache *)cache dirtyLowRootsCount]; +} + ++ (NSUInteger)dirtyHighRootsCountForCache:(id)cache { + return [(iTermProcessCache *)cache dirtyHighRootsCount]; +} + ++ (BOOL)cache:(id)cache isRootHighPriority:(pid_t)rootPID { + return [(iTermProcessCache *)cache isRootHighPriority:rootPID]; +} + ++ (BOOL)cache:(id)cache isTrackingRoot:(pid_t)rootPID { + return [(iTermProcessCache *)cache isTrackingRoot:rootPID]; +} + ++ (void)forceBackgroundRefreshTickForCache:(id)cache { + [(iTermProcessCache *)cache forceBackgroundRefreshTick]; +} + ++ (void)cache:(id)cache registerTestRoot:(pid_t)rootPID { + [(iTermProcessCache *)cache registerTestRoot:rootPID]; +} + ++ (void)cache:(id)cache unregisterTestRoot:(pid_t)rootPID { + [(iTermProcessCache *)cache unregisterTestRoot:rootPID]; +} + ++ (void)cache:(id)cache setForegroundRootPIDs:(NSSet *)foregroundPIDs { + [(iTermProcessCache *)cache setForegroundRootPIDs:foregroundPIDs]; +} + ++ (nullable id)cache:(id)cache monitorForRoot:(pid_t)rootPID { + iTermProcessCache *c = (iTermProcessCache *)cache; + __block iTermProcessMonitor *monitor = nil; + dispatch_sync(c->_lockQueue, ^{ + iTermTrackedRootInfo *info = c->_rootsLQ[@(rootPID)]; + monitor = info.monitor; + }); + return monitor; +} + ++ (BOOL)monitorIsPaused:(id)monitor { + return [(iTermProcessMonitor *)monitor isPaused]; +} + ++ (NSArray *)childMonitorsForMonitor:(id)monitor { + return [(iTermProcessMonitor *)monitor childMonitors]; +} + +@end diff --git a/sources/iTermProcessMonitor+Testing.h b/sources/iTermProcessMonitor+Testing.h new file mode 100644 index 0000000000..4c389600a9 --- /dev/null +++ b/sources/iTermProcessMonitor+Testing.h @@ -0,0 +1,22 @@ +// +// iTermProcessMonitor+Testing.h +// iTerm2SharedARC +// +// Testing-only interface for iTermProcessMonitor. +// + +#import "iTermProcessMonitor.h" + +NS_ASSUME_NONNULL_BEGIN + +@interface iTermProcessMonitor (Testing) + +/// Returns YES if the monitor's dispatch source is currently suspended. +@property (nonatomic, readonly) BOOL isPaused; + +/// Returns the child monitors (for testing child pause state). +@property (nonatomic, readonly) NSArray *childMonitors; + +@end + +NS_ASSUME_NONNULL_END diff --git a/sources/iTermProcessMonitor+Testing.m b/sources/iTermProcessMonitor+Testing.m new file mode 100644 index 0000000000..d06ebdeacc --- /dev/null +++ b/sources/iTermProcessMonitor+Testing.m @@ -0,0 +1,28 @@ +// +// iTermProcessMonitor+Testing.m +// iTerm2SharedARC +// +// Testing-only implementation for iTermProcessMonitor. +// + +#import "iTermProcessMonitor+Testing.h" + +// Forward declare private ivars we need to access +@interface iTermProcessMonitor () { + @public + BOOL _isPaused; + NSMutableArray *_children; +} +@end + +@implementation iTermProcessMonitor (Testing) + +- (BOOL)isPaused { + return _isPaused; +} + +- (NSArray *)childMonitors { + return [_children copy]; +} + +@end From 92ea90a37ce9aa86204edc2ae9034159f0746117 Mon Sep 17 00:00:00 2001 From: chall37 Date: Tue, 3 Feb 2026 15:04:48 -0800 Subject: [PATCH 5/9] Update foreground PIDs on tab change and add testing category to project Notify the process cache of foreground root PIDs when the current tab changes, enabling priority-based process monitoring. Also add the iTermProcessMonitor+Testing category files to the Xcode project. --- iTerm2.xcodeproj/project.pbxproj | 6 ++++++ sources/PseudoTerminal.m | 10 ++++++++++ 2 files changed, 16 insertions(+) diff --git a/iTerm2.xcodeproj/project.pbxproj b/iTerm2.xcodeproj/project.pbxproj index 587e68f53a..db02c8d658 100644 --- a/iTerm2.xcodeproj/project.pbxproj +++ b/iTerm2.xcodeproj/project.pbxproj @@ -724,6 +724,7 @@ 531E71F72229A69C00915960 /* iTermExpressionParser+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 531E71F62229A69C00915960 /* iTermExpressionParser+Private.h */; }; 532755852387821C00C50732 /* iTermProcessMonitor.h in Headers */ = {isa = PBXBuildFile; fileRef = 532755832387821C00C50732 /* iTermProcessMonitor.h */; }; 532755862387821C00C50732 /* iTermProcessMonitor.m in Sources */ = {isa = PBXBuildFile; fileRef = 532755842387821C00C50732 /* iTermProcessMonitor.m */; }; + D7A9E6F12ABC123456789012 /* iTermProcessMonitor+Testing.m in Sources */ = {isa = PBXBuildFile; fileRef = D7A9E6F02ABC123456789011 /* iTermProcessMonitor+Testing.m */; }; 532F942B215DFEF600D509E4 /* iTermCacheableImage.h in Headers */ = {isa = PBXBuildFile; fileRef = 532F9429215DFEF600D509E4 /* iTermCacheableImage.h */; }; 532F942C215DFEF600D509E4 /* iTermCacheableImage.m in Sources */ = {isa = PBXBuildFile; fileRef = 532F942A215DFEF600D509E4 /* iTermCacheableImage.m */; }; 533292A6237E75360027EB49 /* iTermPythonArgumentParserTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 533292A5237E75360027EB49 /* iTermPythonArgumentParserTests.m */; }; @@ -5998,6 +5999,8 @@ 531E71F62229A69C00915960 /* iTermExpressionParser+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "iTermExpressionParser+Private.h"; sourceTree = ""; }; 532755832387821C00C50732 /* iTermProcessMonitor.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = iTermProcessMonitor.h; sourceTree = ""; }; 532755842387821C00C50732 /* iTermProcessMonitor.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = iTermProcessMonitor.m; sourceTree = ""; }; + D7A9E6EF2ABC123456789010 /* iTermProcessMonitor+Testing.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "iTermProcessMonitor+Testing.h"; sourceTree = ""; }; + D7A9E6F02ABC123456789011 /* iTermProcessMonitor+Testing.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "iTermProcessMonitor+Testing.m"; sourceTree = ""; }; 532F9429215DFEF600D509E4 /* iTermCacheableImage.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = iTermCacheableImage.h; sourceTree = ""; }; 532F942A215DFEF600D509E4 /* iTermCacheableImage.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = iTermCacheableImage.m; sourceTree = ""; }; 533292A5237E75360027EB49 /* iTermPythonArgumentParserTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = iTermPythonArgumentParserTests.m; sourceTree = ""; }; @@ -12649,6 +12652,8 @@ A6A802E8226ED20800BC70DC /* iTermPrintGuard.m */, 532755832387821C00C50732 /* iTermProcessMonitor.h */, 532755842387821C00C50732 /* iTermProcessMonitor.m */, + D7A9E6EF2ABC123456789010 /* iTermProcessMonitor+Testing.h */, + D7A9E6F02ABC123456789011 /* iTermProcessMonitor+Testing.m */, 1D468BBF1B056CD600226083 /* iTermProfileSearchToken.m */, A63493F623F2685A0047C31B /* iTermPromise.h */, A63493F723F2685A0047C31B /* iTermPromise.m */, @@ -18854,6 +18859,7 @@ A667C235276AB01F006B4DEF /* iTermMetalUnavailableReason.m in Sources */, A6DB7BAC2CE52BB000EB7973 /* iTermPasteSpecialWindowController.m in Sources */, 532755862387821C00C50732 /* iTermProcessMonitor.m in Sources */, + D7A9E6F12ABC123456789012 /* iTermProcessMonitor+Testing.m in Sources */, A6758565245AA23400827C25 /* iTermSwipeState.m in Sources */, A667C266277A3B29006B4DEF /* PTYTriggerEvaluator.m in Sources */, A6024B82254CE2000036D6CF /* iTermAddTriggerViewController.m in Sources */, diff --git a/sources/PseudoTerminal.m b/sources/PseudoTerminal.m index 71d5dd8f48..18c28df959 100644 --- a/sources/PseudoTerminal.m +++ b/sources/PseudoTerminal.m @@ -4408,6 +4408,16 @@ - (void)windowDidBecomeKey:(NSNotification *)aNotification { [aSession.view setNeedsDisplay:YES]; [aSession useTransparencyDidChange]; } + + // Update process cache with foreground root PIDs for the current tab + NSMutableSet *foregroundPIDs = [NSMutableSet set]; + for (PTYSession *session in [self.currentTab sessions]) { + pid_t pid = session.shell.pid; + if (pid > 0) { + [foregroundPIDs addObject:@(pid)]; + } + } + [[iTermProcessCache sharedInstance] setForegroundRootPIDs:foregroundPIDs]; // Some users report that the first responder isn't always set properly. Let's try to fix that. // This attempt (4/20/13) is to fix bug 2431. if (!self.window.firstResponder.it_preferredFirstResponder) { From e97ca4207e97ff565bd8605acec19072b79b33b8 Mon Sep 17 00:00:00 2001 From: chall37 Date: Wed, 4 Feb 2026 11:47:55 -0800 Subject: [PATCH 6/9] Fix use-after-free crash in iTermProcessMonitor When child monitors are removed from the tree, their dispatch sources must be invalidated before the monitor is deallocated. Without this, the dispatch source is still active when the monitor is freed, causing a crash in _dispatch_queue_xref_dispose. --- sources/iTermProcessMonitor.m | 1 + 1 file changed, 1 insertion(+) diff --git a/sources/iTermProcessMonitor.m b/sources/iTermProcessMonitor.m index 8abf013c52..244543d0af 100644 --- a/sources/iTermProcessMonitor.m +++ b/sources/iTermProcessMonitor.m @@ -116,6 +116,7 @@ - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo depth:(NSInteger)depth { [self addChild:child]; }]; [childrenToRemove enumerateObjectsUsingBlock:^(iTermProcessMonitor * _Nonnull child, NSUInteger idx, BOOL * _Nonnull stop) { + [child invalidate]; [self removeChild:child]; }]; if (childrenToAdd.count || childrenToRemove.count) { From 964abcdb4f40e0f35d82dd1d58c84e213eb41ad6 Mon Sep 17 00:00:00 2001 From: chall37 Date: Wed, 4 Feb 2026 13:40:18 -0800 Subject: [PATCH 7/9] Fix pause state not recorded before dispatch source exists When pauseMonitoring was called before setProcessInfo (i.e., before a dispatch source existed), the paused state was not recorded. Later when setProcessInfo created the source, it would auto-resume, bypassing the intended suspension for background tabs. Changes: - pauseMonitoring/resumeMonitoring now record _isPaused even without a source - setProcessInfo respects pre-recorded pause state when creating sources - Added unit tests for pre-source pause behavior - Added test for child removal invalidation --- ModernTests/iTermProcessMonitorTests.swift | 335 +++++++++++++++++++++ sources/iTermProcessMonitor.m | 41 ++- 2 files changed, 362 insertions(+), 14 deletions(-) diff --git a/ModernTests/iTermProcessMonitorTests.swift b/ModernTests/iTermProcessMonitorTests.swift index 70be6466e6..6ef49b0bca 100644 --- a/ModernTests/iTermProcessMonitorTests.swift +++ b/ModernTests/iTermProcessMonitorTests.swift @@ -219,6 +219,91 @@ final class iTermProcessMonitorTests: XCTestCase { XCTAssertEqual(parentInfo.children.count, 2) } + func testSetProcessInfo_RemovesChild_InvalidatesChildMonitor() { + // Given: A monitor, first set process info without children, then add a child + let parentPID = getpid() + let childPID: pid_t = 99991 + + dataSource.processNames[parentPID] = "xctest" + + // First: set up monitor with no children + let initialCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()) + ]) + + guard let initialInfo = initialCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create initial process info") + return + } + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) + + testQueue.sync { + _ = monitor.setProcessInfo(initialInfo) + } + + // Pause the monitor (required for child monitors to be properly tracked with fake PIDs) + testQueue.sync { + monitor.pauseMonitoring() + } + + // Then: add a child + dataSource.processNames[childPID] = "child" + + let withChildCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (childPID, parentPID) + ]) + + guard let withChildInfo = withChildCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create process info with child") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(withChildInfo) + } + + // Verify child was created while still paused + var childMonitor: iTermProcessMonitor? + testQueue.sync { + let children = childMonitors(for: monitor) + XCTAssertEqual(children.count, 1, "Should have one child monitor") + childMonitor = children.first as? iTermProcessMonitor + } + + guard let capturedChild = childMonitor else { + XCTFail("Expected child monitor") + return + } + + // Resume for the rest of the test + testQueue.sync { + monitor.resumeMonitoring() + } + + // When: Update process info to remove the child + let withoutChildCollection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()) + ]) + + guard let withoutChildInfo = withoutChildCollection.info(forProcessID: parentPID) else { + XCTFail("Failed to create process info without child") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(withoutChildInfo) + } + + // Then: Removed child should be invalidated (processInfo cleared) + XCTAssertNil(capturedChild.processInfo, "Removed child monitor should be invalidated") + + testQueue.sync { + monitor.invalidate() + } + } + func testSetProcessInfo_TrackedRootPID_PropagatedToChildren() { // Given: A process tree let rootPID: pid_t = 12345 @@ -648,6 +733,255 @@ final class iTermProcessMonitorTests: XCTestCase { } } + // MARK: - 5b. Pre-Source Pause Tests (pauseMonitoring before setProcessInfo) + // These tests verify the fix for the bug where pauseMonitoring called before + // setProcessInfo: (i.e., before a dispatch source exists) would not record the + // paused state, causing the source to auto-resume when created. + + func testPauseBeforeSetProcessInfo_SourceStartsPaused() { + // This tests the core fix: calling pauseMonitoring before setProcessInfo: + // should record the paused state, and the dispatch source should NOT auto-resume. + + // Given: A monitor with NO process info yet (no dispatch source) + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: 0) + + // When: Pause BEFORE setProcessInfo creates a dispatch source + testQueue.sync { + monitor.pauseMonitoring() + } + + // Verify isPaused is true even without a source + var pausedBeforeSource = false + testQueue.sync { + pausedBeforeSource = isMonitorPaused(monitor) + } + XCTAssertTrue(pausedBeforeSource, "isPaused should be true even before dispatch source exists") + + // Now set process info to create the dispatch source + let pid = getpid() + dataSource.processNames[pid] = "xctest" + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } + + // Then: Monitor should STILL be paused after source creation + var pausedAfterSource = false + testQueue.sync { + pausedAfterSource = isMonitorPaused(monitor) + } + XCTAssertTrue(pausedAfterSource, "Monitor should remain paused after setProcessInfo creates source") + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } + + func testPauseBeforeSetProcessInfo_ResumeWorks() { + // Verifies that after pausing before source creation, resumeMonitoring works correctly. + + // Given: A monitor paused before setProcessInfo + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: 0) + + testQueue.sync { + monitor.pauseMonitoring() + } + + // Set process info to create source (remains paused) + let pid = getpid() + dataSource.processNames[pid] = "xctest" + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } + + // When: Resume the monitor + testQueue.sync { + monitor.resumeMonitoring() + } + + // Then: Should no longer be paused + var pausedAfterResume = false + testQueue.sync { + pausedAfterResume = isMonitorPaused(monitor) + } + XCTAssertFalse(pausedAfterResume, "Monitor should be running after resumeMonitoring") + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } + + func testResumeBeforeSetProcessInfo_ClearsPausedState() { + // Verifies that calling resume before setProcessInfo clears the paused state, + // so the source will auto-resume when created (normal behavior). + + // Given: A monitor that was paused then resumed, all before setProcessInfo + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: 0) + + testQueue.sync { + monitor.pauseMonitoring() + monitor.resumeMonitoring() + } + + // Verify isPaused is false before source creation + var pausedBeforeSource = false + testQueue.sync { + pausedBeforeSource = isMonitorPaused(monitor) + } + XCTAssertFalse(pausedBeforeSource, "isPaused should be false after resume (before source)") + + // When: Set process info to create source + let pid = getpid() + dataSource.processNames[pid] = "xctest" + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } + + // Then: Monitor should NOT be paused (source auto-resumed) + var pausedAfterSource = false + testQueue.sync { + pausedAfterSource = isMonitorPaused(monitor) + } + XCTAssertFalse(pausedAfterSource, "Monitor should be running after setProcessInfo (was not pre-paused)") + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } + + func testPauseBeforeSetProcessInfo_ChildrenAlsoPaused() { + // Verifies that when a monitor is pre-paused (before source creation), + // any children created by setProcessInfo are also paused. + + // Given: A monitor paused before setProcessInfo + let parentPID = getpid() + let childPID: pid_t = 99991 + + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) + + testQueue.sync { + monitor.pauseMonitoring() + } + + // When: Set process info with children + dataSource.processNames[parentPID] = "xctest" + dataSource.processNames[childPID] = "child" + + let collection = makeProcessCollection(dataSource: dataSource, processes: [ + (parentPID, getppid()), + (childPID, parentPID) + ]) + + guard let parentInfo = collection.info(forProcessID: parentPID) else { + XCTFail("Failed to create parent process info") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(parentInfo) + } + + // Then: Both parent and children should be paused + testQueue.sync { + XCTAssertTrue(isMonitorPaused(monitor), "Parent should be paused") + + let children = childMonitors(for: monitor) + XCTAssertEqual(children.count, 1, "Should have one child") + + if let child = children.first { + XCTAssertTrue(isMonitorPaused(child), "Child should also be paused when parent was pre-paused") + } + } + + // When: Resume + testQueue.sync { + monitor.resumeMonitoring() + } + + // Then: Both should be running + testQueue.sync { + XCTAssertFalse(isMonitorPaused(monitor), "Parent should be running after resume") + + let children = childMonitors(for: monitor) + if let child = children.first { + XCTAssertFalse(isMonitorPaused(child), "Child should be running after resume") + } + } + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } + + func testPauseBeforeSetProcessInfo_MultiplePauseResumeBeforeSource() { + // Edge case: multiple pause/resume cycles before source creation. + + // Given: A monitor with no process info + let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: 0) + + // When: Multiple pause/resume cycles + testQueue.sync { + monitor.pauseMonitoring() // paused + monitor.resumeMonitoring() // not paused + monitor.pauseMonitoring() // paused + monitor.pauseMonitoring() // still paused (idempotent) + monitor.resumeMonitoring() // not paused + monitor.resumeMonitoring() // still not paused (idempotent) + monitor.pauseMonitoring() // paused (final state) + } + + // Then: Final state should be paused + var isPaused = false + testQueue.sync { + isPaused = isMonitorPaused(monitor) + } + XCTAssertTrue(isPaused, "Final state should be paused") + + // When: Create source + let pid = getpid() + dataSource.processNames[pid] = "xctest" + let collection = makeProcessCollection(dataSource: dataSource, processes: [(pid, getppid())]) + guard let processInfo = collection.info(forProcessID: pid) else { + XCTFail("Failed to create process info") + return + } + + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } + + // Then: Should still be paused + testQueue.sync { + XCTAssertTrue(isMonitorPaused(monitor), "Should remain paused after source creation") + } + + // Cleanup + testQueue.sync { + monitor.invalidate() + } + } + // MARK: - 6. Invalidation Tests func testInvalidate_ClearsProcessInfo() { @@ -825,4 +1159,5 @@ final class iTermProcessMonitorTests: XCTestCase { monitor.invalidate() } } + } diff --git a/sources/iTermProcessMonitor.m b/sources/iTermProcessMonitor.m index 244543d0af..e20a00759c 100644 --- a/sources/iTermProcessMonitor.m +++ b/sources/iTermProcessMonitor.m @@ -17,9 +17,9 @@ @interface iTermProcessMonitor() @end @implementation iTermProcessMonitor { - dispatch_source_t _source; - NSMutableArray *_children; - BOOL _isPaused; + dispatch_source_t _source; // Access on _queue only + NSMutableArray *_children; // Access on _queue only + BOOL _isPaused; // Access on _queue only } - (instancetype)initWithQueue:(dispatch_queue_t)queue @@ -40,10 +40,12 @@ - (instancetype)initWithQueue:(dispatch_queue_t)queue return [self initWithQueue:queue callback:callback trackedRootPID:0]; } +// Called on _queue - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo { return [self setProcessInfo:processInfo depth:0]; } +// Called on _queue - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo depth:(NSInteger)depth { if (![iTermAdvancedSettingsModel fastForegroundJobUpdates]) { return NO; @@ -81,7 +83,11 @@ - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo depth:(NSInteger)depth { dispatch_source_set_event_handler(_source, ^{ [weakSelf handleEvent]; }); - dispatch_resume(_source); + // Only resume if not paused. pauseMonitoring may have been called before + // we had a source, in which case _isPaused is already set. + if (!_isPaused) { + dispatch_resume(_source); + } } NSMutableArray *childrenToAdd = [NSMutableArray array]; @@ -126,6 +132,7 @@ - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo depth:(NSInteger)depth { return changed; } +// Called on _queue - (iTermProcessMonitor *)childForProcessInfo:(iTermProcessInfo *)info { const pid_t pid = info.processID; return [_children objectPassingTest:^BOOL(iTermProcessMonitor *element, NSUInteger index, BOOL *stop) { @@ -166,32 +173,38 @@ - (void)invalidate { // Called on _queue - (void)pauseMonitoring { - if (_source == nil || _isPaused) { + if (_isPaused) { return; } - DLog(@"Pause monitoring process %@", _processInfo); - dispatch_suspend(_source); _isPaused = YES; - - // Recursively pause children + // Recursively pause children (do this even if _source is nil, since children may have sources) for (iTermProcessMonitor *child in _children) { [child pauseMonitoring]; } + if (_source == nil) { + // No source yet; _isPaused is recorded so setProcessInfo: won't auto-resume. + return; + } + DLog(@"Pause monitoring process %@", _processInfo); + dispatch_suspend(_source); } // Called on _queue - (void)resumeMonitoring { - if (_source == nil || !_isPaused) { + if (!_isPaused) { return; } - DLog(@"Resume monitoring process %@", _processInfo); - dispatch_resume(_source); _isPaused = NO; - - // Recursively resume children + // Recursively resume children (do this even if _source is nil, since children may have sources) for (iTermProcessMonitor *child in _children) { [child resumeMonitoring]; } + if (_source == nil) { + // No source yet; _isPaused is cleared so setProcessInfo: will resume normally. + return; + } + DLog(@"Resume monitoring process %@", _processInfo); + dispatch_resume(_source); } // Called on _queue From 6f9c2041553b305bf4e9978473abbeac92b8ab41 Mon Sep 17 00:00:00 2001 From: chall37 Date: Wed, 4 Feb 2026 13:45:10 -0800 Subject: [PATCH 8/9] Add thread-safety annotations to iTermProcessCache Document queue affinity for setupCoalescer, processIsAlive, and backgroundRefreshTick methods. --- sources/iTermProcessCache.m | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sources/iTermProcessCache.m b/sources/iTermProcessCache.m index 8539334c9b..db992938c2 100644 --- a/sources/iTermProcessCache.m +++ b/sources/iTermProcessCache.m @@ -470,6 +470,7 @@ - (void)reallyUpdate { #pragma mark - Coalescer +// Called during init - (void)setupCoalescer { _coalescer = dispatch_source_create(DISPATCH_SOURCE_TYPE_DATA_OR, 0, 0, _workQueue); @@ -663,7 +664,7 @@ - (void)evictUnseenNodesForRoot:(pid_t)rootPid epoch:(NSUInteger)epoch { // A more sophisticated implementation could track (pid -> lastSeenEpoch) in cachedDescendants } -// Check if a process is still alive using kill(pid, 0) +// Any queue - Check if a process is still alive using kill(pid, 0) // Returns YES if process exists, NO if confirmed dead - (BOOL)processIsAlive:(pid_t)pid { if (pid <= 0) { @@ -756,7 +757,7 @@ - (void)setForegroundRootPIDsLQ:(NSSet *)foregroundPIDs { #pragma mark - Amortized Background Refresh -// Called by 0.5s cadence timer - refresh 1-2 background roots per tick +// Any queue - Called by 0.5s cadence timer - refresh 1-2 background roots per tick - (void)backgroundRefreshTick { __block NSArray *toRefresh = nil; __block iTermProcessCollection *collection = nil; From 7185fecfdf84e3fb58ec78a8bff902276da20cf1 Mon Sep 17 00:00:00 2001 From: chall37 Date: Wed, 4 Feb 2026 14:16:15 -0800 Subject: [PATCH 9/9] Add ITAssertOnQueue macro and enforce queue affinity in iTermProcessMonitor Add a new ITAssertOnQueue macro that wraps dispatch_assert_queue() for debug builds. Use it in iTermProcessMonitor to enforce that methods marked "Called on _queue" are actually called on that queue. Fix tests to call setProcessInfo on the correct queue. --- ModernTests/iTermProcessMonitorTests.swift | 80 +++++++++++++++++----- sources/DebugLogging.h | 8 +++ sources/iTermProcessMonitor.m | 9 +++ 3 files changed, 78 insertions(+), 19 deletions(-) diff --git a/ModernTests/iTermProcessMonitorTests.swift b/ModernTests/iTermProcessMonitorTests.swift index 6ef49b0bca..d0b63e6101 100644 --- a/ModernTests/iTermProcessMonitorTests.swift +++ b/ModernTests/iTermProcessMonitorTests.swift @@ -133,7 +133,10 @@ final class iTermProcessMonitorTests: XCTestCase { let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) // When - let changed = monitor.setProcessInfo(processInfo) + var changed = false + testQueue.sync { + changed = monitor.setProcessInfo(processInfo) + } // Then XCTAssertTrue(changed) @@ -152,10 +155,15 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) - _ = monitor.setProcessInfo(processInfo) + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } // When - let changed = monitor.setProcessInfo(processInfo) + var changed = false + testQueue.sync { + changed = monitor.setProcessInfo(processInfo) + } // Then XCTAssertFalse(changed) @@ -173,11 +181,16 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) - _ = monitor.setProcessInfo(processInfo) + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } XCTAssertNotNil(monitor.processInfo) // When - let changed = monitor.setProcessInfo(nil as iTermProcessInfo?) + var changed = false + testQueue.sync { + changed = monitor.setProcessInfo(nil as iTermProcessInfo?) + } // Then XCTAssertTrue(changed) @@ -211,7 +224,10 @@ final class iTermProcessMonitorTests: XCTestCase { let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) // When - let changed = monitor.setProcessInfo(parentInfo) + var changed = false + testQueue.sync { + changed = monitor.setProcessInfo(parentInfo) + } // Then XCTAssertTrue(changed) @@ -327,7 +343,9 @@ final class iTermProcessMonitorTests: XCTestCase { let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: rootPID) // When - _ = monitor.setProcessInfo(parentInfo) + testQueue.sync { + _ = monitor.setProcessInfo(parentInfo) + } // Then XCTAssertEqual(monitor.trackedRootPID, rootPID) @@ -348,7 +366,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) - _ = monitor.setProcessInfo(processInfo) + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } // When: Pause multiple times testQueue.sync { @@ -375,7 +395,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) - _ = monitor.setProcessInfo(processInfo) + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } // When: Resume multiple times without pausing first testQueue.sync { @@ -427,7 +449,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) - _ = monitor.setProcessInfo(initialInfo) + testQueue.sync { + _ = monitor.setProcessInfo(initialInfo) + } // Pause the parent monitor testQueue.sync { @@ -498,7 +522,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) - _ = monitor.setProcessInfo(initialInfo) + testQueue.sync { + _ = monitor.setProcessInfo(initialInfo) + } // Pause the monitor tree testQueue.sync { @@ -573,7 +599,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) - _ = monitor.setProcessInfo(initialInfo) + testQueue.sync { + _ = monitor.setProcessInfo(initialInfo) + } // Pause the parent monitor testQueue.sync { @@ -633,7 +661,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) - _ = monitor.setProcessInfo(initialInfo) + testQueue.sync { + _ = monitor.setProcessInfo(initialInfo) + } // Pause the parent testQueue.sync { @@ -699,7 +729,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) - _ = monitor.setProcessInfo(initialInfo) + testQueue.sync { + _ = monitor.setProcessInfo(initialInfo) + } // Parent is NOT paused @@ -996,7 +1028,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) - _ = monitor.setProcessInfo(processInfo) + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } XCTAssertNotNil(monitor.processInfo) // When @@ -1020,7 +1054,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) - _ = monitor.setProcessInfo(processInfo) + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } testQueue.sync { monitor.pauseMonitoring() @@ -1046,7 +1082,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: pid) - _ = monitor.setProcessInfo(processInfo) + testQueue.sync { + _ = monitor.setProcessInfo(processInfo) + } // When: Invalidate multiple times testQueue.sync { @@ -1080,7 +1118,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) - _ = monitor.setProcessInfo(parentInfo) + testQueue.sync { + _ = monitor.setProcessInfo(parentInfo) + } // When: Pause and resume multiple times for _ in 0..<5 { @@ -1117,7 +1157,9 @@ final class iTermProcessMonitorTests: XCTestCase { } let monitor = iTermProcessMonitor(queue: testQueue, callback: makeCallback(), trackedRootPID: parentPID) - _ = monitor.setProcessInfo(initialInfo) + testQueue.sync { + _ = monitor.setProcessInfo(initialInfo) + } // Pause testQueue.sync { diff --git a/sources/DebugLogging.h b/sources/DebugLogging.h index a383762469..13e9230fd9 100644 --- a/sources/DebugLogging.h +++ b/sources/DebugLogging.h @@ -32,6 +32,14 @@ extern BOOL gDebugLogging; #define ITDebugAssert(condition) ((void)(condition)) #endif +// Assert that we're currently executing on the specified dispatch queue. +// Uses dispatch_assert_queue() which provides clear error messages. +#if ITERM_DEBUG +#define ITAssertOnQueue(queue) dispatch_assert_queue(queue) +#else +#define ITAssertOnQueue(queue) ((void)(queue)) +#endif + #if !ITERM_DEBUG && USE_STOPWATCH #define STOPWATCH_START(name) \ NSTimeInterval start_##name = [NSDate timeIntervalSinceReferenceDate]; \ diff --git a/sources/iTermProcessMonitor.m b/sources/iTermProcessMonitor.m index e20a00759c..584497a58c 100644 --- a/sources/iTermProcessMonitor.m +++ b/sources/iTermProcessMonitor.m @@ -42,11 +42,13 @@ - (instancetype)initWithQueue:(dispatch_queue_t)queue // Called on _queue - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo { + ITAssertOnQueue(_queue); return [self setProcessInfo:processInfo depth:0]; } // Called on _queue - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo depth:(NSInteger)depth { + ITAssertOnQueue(_queue); if (![iTermAdvancedSettingsModel fastForegroundJobUpdates]) { return NO; } @@ -134,6 +136,7 @@ - (BOOL)setProcessInfo:(iTermProcessInfo *)processInfo depth:(NSInteger)depth { // Called on _queue - (iTermProcessMonitor *)childForProcessInfo:(iTermProcessInfo *)info { + ITAssertOnQueue(_queue); const pid_t pid = info.processID; return [_children objectPassingTest:^BOOL(iTermProcessMonitor *element, NSUInteger index, BOOL *stop) { return element.processInfo.processID == pid; @@ -142,6 +145,7 @@ - (iTermProcessMonitor *)childForProcessInfo:(iTermProcessInfo *)info { // Called on _queue - (void)handleEvent { + ITAssertOnQueue(_queue); const dispatch_source_proc_flags_t flags = (dispatch_source_proc_flags_t)dispatch_source_get_data(_source); _callback(self, flags); if (flags & DISPATCH_PROC_EXIT) { @@ -151,6 +155,7 @@ - (void)handleEvent { // Called on _queue - (void)invalidate { + ITAssertOnQueue(_queue); if (_source == nil) { return; } @@ -173,6 +178,7 @@ - (void)invalidate { // Called on _queue - (void)pauseMonitoring { + ITAssertOnQueue(_queue); if (_isPaused) { return; } @@ -191,6 +197,7 @@ - (void)pauseMonitoring { // Called on _queue - (void)resumeMonitoring { + ITAssertOnQueue(_queue); if (!_isPaused) { return; } @@ -209,12 +216,14 @@ - (void)resumeMonitoring { // Called on _queue - (void)addChild:(iTermProcessMonitor *)child { + ITAssertOnQueue(_queue); [_children addObject:child]; child.parent = self; } // Called on _queue - (void)removeChild:(iTermProcessMonitor *)child { + ITAssertOnQueue(_queue); if (child.parent == self) { [_children removeObject:child]; child.parent = nil;