From c89ff1d3293a67fc2953c0addb0258ee39d55ecd Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sun, 16 Nov 2025 11:06:34 +0000 Subject: [PATCH] Fix: prevent memory leak from failed services accumulating in ServiceManager - Add cleanupTimeoutId field to ServiceHandle interface - Implement scheduleCleanup method to automatically remove failed services after 5 minutes - Schedule cleanup when services fail during runtime or timeout during stopService - Clear cleanup timeouts when services are manually stopped - All existing tests pass --- src/integrations/terminal/ServiceManager.ts | 43 +++++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/integrations/terminal/ServiceManager.ts b/src/integrations/terminal/ServiceManager.ts index 8435ba3129..7b876f143d 100644 --- a/src/integrations/terminal/ServiceManager.ts +++ b/src/integrations/terminal/ServiceManager.ts @@ -27,6 +27,7 @@ export interface ServiceHandle { healthCheckUrl?: string healthCheckIntervalMs?: number healthCheckIntervalId?: NodeJS.Timeout + cleanupTimeoutId?: NodeJS.Timeout } /** @@ -122,6 +123,11 @@ export class ServiceManager { } this.notifyStatusChange(serviceHandle) + + // Schedule cleanup for failed services to prevent memory leak + if (serviceHandle.status === "failed") { + this.scheduleCleanup(serviceHandle) + } }, } @@ -159,6 +165,12 @@ export class ServiceManager { service.status = "stopping" this.notifyStatusChange(service) + // Clear any scheduled cleanup since we're manually stopping + if (service.cleanupTimeoutId) { + clearTimeout(service.cleanupTimeoutId) + service.cleanupTimeoutId = undefined + } + // Stop health check if (service.healthCheckIntervalId) { clearInterval(service.healthCheckIntervalId) @@ -184,7 +196,7 @@ export class ServiceManager { return } - // If timeout, mark as failed but keep in list + // If timeout, mark as failed and schedule cleanup if (waitedTime >= maxWaitTime) { clearInterval(interval) // Check if process is really still running @@ -192,14 +204,15 @@ export class ServiceManager { try { // Try sending signal 0 to check if process exists (won't terminate process) process.kill(service.pid, 0) - // If process still exists, mark as failed status, keep in list + // If process still exists, mark as failed status and schedule cleanup service.status = "failed" service.logs.push( `[ServiceManager] Warning: Service did not terminate within ${maxWaitTime}ms. Process may still be running.`, ) this.notifyStatusChange(service) + this.scheduleCleanup(service) console.warn( - `[ServiceManager] Service ${serviceId} (PID: ${service.pid}) did not terminate within timeout. Marked as failed but kept in list.`, + `[ServiceManager] Service ${serviceId} (PID: ${service.pid}) did not terminate within timeout. Marked as failed and scheduled for cleanup.`, ) } catch (error) { // Process doesn't exist (errno === ESRCH), means it has terminated @@ -207,12 +220,13 @@ export class ServiceManager { this.notifyStatusChange(service) } } else { - // No PID, mark as failed + // No PID, mark as failed and schedule cleanup service.status = "failed" service.logs.push( `[ServiceManager] Warning: Service did not terminate within ${maxWaitTime}ms. No PID available.`, ) this.notifyStatusChange(service) + this.scheduleCleanup(service) } resolve(undefined) } @@ -272,6 +286,23 @@ export class ServiceManager { } } + /** + * Schedule cleanup of a failed service after a delay + * This prevents memory leaks from failed services accumulating indefinitely + */ + private static scheduleCleanup(serviceHandle: ServiceHandle, delayMs: number = 300000): void { + // Clear any existing cleanup timeout + if (serviceHandle.cleanupTimeoutId) { + clearTimeout(serviceHandle.cleanupTimeoutId) + } + + // Schedule cleanup after delay (default 5 minutes) + serviceHandle.cleanupTimeoutId = setTimeout(() => { + this.services.delete(serviceHandle.serviceId) + console.log(`[ServiceManager] Cleaned up failed service ${serviceHandle.serviceId} after ${delayMs}ms`) + }, delayMs) as unknown as NodeJS.Timeout + } + /** * Start health check */ @@ -282,6 +313,10 @@ export class ServiceManager { clearInterval(serviceHandle.healthCheckIntervalId) serviceHandle.healthCheckIntervalId = undefined } + // If failed, schedule cleanup + if (serviceHandle.status === "failed") { + this.scheduleCleanup(serviceHandle) + } return }