Browse Source

fix(companion): dedup process exit listener across re-inits

CompanionManager.onLoad() registered a new process 'exit' listener every
time it ran and never removed it. The plugin function can re-run
(config.update() → Instance.dispose()), constructing a fresh manager each
time, so listeners stacked up and each retained its previous manager (and
its detached child reference), eventually tripping MaxListenersExceeded.

Track the live listener at module level (which survives re-inits) and
remove the previous one before registering a new one, bounding the
process to a single companion exit listener. onExit() now also removes
its own listener so a manually-disposed manager leaves none behind.

Follow-up to the resource audit in #597 / #600.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Zerdeşt Taifour 1 month ago
parent
commit
7180ff72a9
2 changed files with 49 additions and 1 deletions
  1. 15 0
      src/companion/manager.test.ts
  2. 34 1
      src/companion/manager.ts

+ 15 - 0
src/companion/manager.test.ts

@@ -210,6 +210,21 @@ describe('CompanionManager', () => {
     expect(readState().sessions[0].active_agents).toEqual(['fixer', 'fixer']);
   });
 
+  it('keeps at most one process exit listener across reloads', () => {
+    const baseline = process.listenerCount('exit');
+    const managers: CompanionManager[] = [];
+    for (let i = 0; i < 5; i++) {
+      const m = make(`reload-${i}`);
+      m.onLoad();
+      managers.push(m);
+    }
+    // Re-inits must dedup the exit listener rather than stacking one each time.
+    expect(process.listenerCount('exit')).toBeLessThanOrEqual(baseline + 1);
+    // onExit releases the live listener again.
+    managers.at(-1)?.onExit();
+    expect(process.listenerCount('exit')).toBeLessThanOrEqual(baseline);
+  });
+
   it('removes its entry on exit', () => {
     const m = make('sess-a', '/a');
     const m2 = make('sess-b', '/b');

+ 34 - 1
src/companion/manager.ts

@@ -12,6 +12,14 @@ import * as path from 'node:path';
 import type { CompanionConfig } from '../config/schema';
 import { log } from '../utils/logger';
 
+// Only one companion `process.on('exit')` listener should be live per process.
+// The plugin function can re-run (config.update() → Instance.dispose()),
+// constructing a fresh CompanionManager each time; without deduping, every
+// re-init would leak another exit listener and retain the previous manager
+// (and its detached child reference). Module-level state survives re-inits
+// because the module itself is not re-evaluated.
+let activeExitListener: (() => void) | null = null;
+
 interface CompanionSession {
   session_id: string;
   cwd: string;
@@ -144,6 +152,7 @@ export class CompanionManager {
   private readonly busyAgentSessions = new Map<string, string>();
   private readonly config?: CompanionConfig;
   private companionProcess: ChildProcess | null = null;
+  private exitListener: (() => void) | null = null;
 
   constructor(sessionId: string, cwd: string, config?: CompanionConfig) {
     this.id = sessionId;
@@ -163,11 +172,28 @@ export class CompanionManager {
       } catch {}
       return;
     }
-    process.on('exit', () => this.onExit());
+    this.registerExitListener();
     this.flush();
     this.spawnIfAvailable();
   }
 
+  /**
+   * Register a single process `exit` listener, replacing any previously
+   * registered companion listener so re-inits don't stack listeners (and so
+   * the prior manager + its child reference become collectable).
+   */
+  private registerExitListener(): void {
+    if (activeExitListener) {
+      try {
+        process.removeListener('exit', activeExitListener);
+      } catch {}
+    }
+    const listener = () => this.onExit();
+    process.on('exit', listener);
+    activeExitListener = listener;
+    this.exitListener = listener;
+  }
+
   /**
    * Feed every session.status event here, with the agent name resolved
    * from sessionAgentMap. Orchestrator sessions drive overall status;
@@ -224,6 +250,13 @@ export class CompanionManager {
 
   onExit(): void {
     if (this.config?.enabled !== true) return;
+    if (this.exitListener) {
+      try {
+        process.removeListener('exit', this.exitListener);
+      } catch {}
+      if (activeExitListener === this.exitListener) activeExitListener = null;
+      this.exitListener = null;
+    }
     if (this.companionProcess) {
       try {
         this.companionProcess.kill();