mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-08 16:39:42 +02:00
fix: memory leaks and error handling improvements
- execution-bridge: track and cleanup retry timers to prevent memory leaks - model-selector: add exhaustive type check for ModelTier - ralph-tracker: fix division by zero when summary.total is 0 - respawn-controller: remove event listeners on stop to prevent memory leaks - screen-manager: use Promise.allSettled for better error handling in stats - session-manager: use Promise.allSettled for stopAllSessions resilience Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
+24
-2
@@ -211,6 +211,7 @@ export class ExecutionBridge extends EventEmitter {
|
|||||||
|
|
||||||
private _pollTimer: NodeJS.Timeout | null = null;
|
private _pollTimer: NodeJS.Timeout | null = null;
|
||||||
private _groupTimeoutTimers: Map<number, NodeJS.Timeout> = new Map();
|
private _groupTimeoutTimers: Map<number, NodeJS.Timeout> = new Map();
|
||||||
|
private _retryTimers: Map<string, NodeJS.Timeout> = new Map();
|
||||||
private _runningTasks: Map<string, { startedAt: number; sessionId?: string }> = new Map();
|
private _runningTasks: Map<string, { startedAt: number; sessionId?: string }> = new Map();
|
||||||
|
|
||||||
private _history: ExecutionHistoryEntry[] = [];
|
private _history: ExecutionHistoryEntry[] = [];
|
||||||
@@ -232,6 +233,12 @@ export class ExecutionBridge extends EventEmitter {
|
|||||||
});
|
});
|
||||||
|
|
||||||
this._scheduler.on('groupCompleted', group => {
|
this._scheduler.on('groupCompleted', group => {
|
||||||
|
// Clear the group timeout timer to prevent memory leak
|
||||||
|
const timer = this._groupTimeoutTimers.get(group.groupNumber);
|
||||||
|
if (timer) {
|
||||||
|
clearTimeout(timer);
|
||||||
|
this._groupTimeoutTimers.delete(group.groupNumber);
|
||||||
|
}
|
||||||
this.emit('groupCompleted', {
|
this.emit('groupCompleted', {
|
||||||
groupNumber: group.groupNumber,
|
groupNumber: group.groupNumber,
|
||||||
status: group.status,
|
status: group.status,
|
||||||
@@ -641,12 +648,14 @@ export class ExecutionBridge extends EventEmitter {
|
|||||||
});
|
});
|
||||||
|
|
||||||
if (willRetry) {
|
if (willRetry) {
|
||||||
// Schedule retry
|
// Schedule retry with tracked timer
|
||||||
setTimeout(() => {
|
const retryTimer = setTimeout(() => {
|
||||||
|
this._retryTimers.delete(task.id);
|
||||||
task.status = 'pending';
|
task.status = 'pending';
|
||||||
task.error = undefined;
|
task.error = undefined;
|
||||||
this._runningTasks.delete(task.id);
|
this._runningTasks.delete(task.id);
|
||||||
}, TASK_RETRY_DELAY_MS);
|
}, TASK_RETRY_DELAY_MS);
|
||||||
|
this._retryTimers.set(task.id, retryTimer);
|
||||||
} else {
|
} else {
|
||||||
// Mark as permanently failed
|
// Mark as permanently failed
|
||||||
this._scheduler.updateTaskStatus(task.id, 'failed', error);
|
this._scheduler.updateTaskStatus(task.id, 'failed', error);
|
||||||
@@ -667,6 +676,13 @@ export class ExecutionBridge extends EventEmitter {
|
|||||||
const groupNum = this.findTaskGroup(taskId);
|
const groupNum = this.findTaskGroup(taskId);
|
||||||
if (groupNum === null) return;
|
if (groupNum === null) return;
|
||||||
|
|
||||||
|
// Clear any pending retry timer for this task
|
||||||
|
const retryTimer = this._retryTimers.get(taskId);
|
||||||
|
if (retryTimer) {
|
||||||
|
clearTimeout(retryTimer);
|
||||||
|
this._retryTimers.delete(taskId);
|
||||||
|
}
|
||||||
|
|
||||||
this._scheduler.updateTaskStatus(taskId, 'completed');
|
this._scheduler.updateTaskStatus(taskId, 'completed');
|
||||||
this._runningTasks.delete(taskId);
|
this._runningTasks.delete(taskId);
|
||||||
}
|
}
|
||||||
@@ -790,6 +806,12 @@ export class ExecutionBridge extends EventEmitter {
|
|||||||
}
|
}
|
||||||
this._groupTimeoutTimers.clear();
|
this._groupTimeoutTimers.clear();
|
||||||
|
|
||||||
|
// Clear any pending retry timers
|
||||||
|
for (const timer of this._retryTimers.values()) {
|
||||||
|
clearTimeout(timer);
|
||||||
|
}
|
||||||
|
this._retryTimers.clear();
|
||||||
|
|
||||||
this._scheduler.reset();
|
this._scheduler.reset();
|
||||||
this._runningTasks.clear();
|
this._runningTasks.clear();
|
||||||
this._status = 'idle';
|
this._status = 'idle';
|
||||||
|
|||||||
@@ -288,6 +288,12 @@ export class ModelSelector extends EventEmitter {
|
|||||||
case 'opus': return 5.0; // ~5x more expensive
|
case 'opus': return 5.0; // ~5x more expensive
|
||||||
case 'sonnet': return 1.0; // baseline
|
case 'sonnet': return 1.0; // baseline
|
||||||
case 'haiku': return 0.04; // ~25x cheaper
|
case 'haiku': return 0.04; // ~25x cheaper
|
||||||
|
default: {
|
||||||
|
// Exhaustive check - if new ModelTier added, this will catch it
|
||||||
|
const _exhaustive: never = model;
|
||||||
|
console.warn(`[ModelSelector] Unknown model tier: ${_exhaustive}, using sonnet multiplier`);
|
||||||
|
return 1.0;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -931,7 +931,7 @@ export class RalphTracker extends EventEmitter {
|
|||||||
// Prevent unbounded line buffer growth from very long lines
|
// Prevent unbounded line buffer growth from very long lines
|
||||||
if (this._lineBuffer.length > MAX_LINE_BUFFER_SIZE) {
|
if (this._lineBuffer.length > MAX_LINE_BUFFER_SIZE) {
|
||||||
// Truncate to last portion to preserve recent data
|
// Truncate to last portion to preserve recent data
|
||||||
this._lineBuffer = this._lineBuffer.slice(-MAX_LINE_BUFFER_SIZE / 2);
|
this._lineBuffer = this._lineBuffer.slice(-Math.floor(MAX_LINE_BUFFER_SIZE / 2));
|
||||||
}
|
}
|
||||||
|
|
||||||
// Process complete lines
|
// Process complete lines
|
||||||
@@ -2614,12 +2614,14 @@ export class RalphTracker extends EventEmitter {
|
|||||||
recommendations.push('More tasks have failed than completed. Review approach and consider plan adjustment.');
|
recommendations.push('More tasks have failed than completed. Review approach and consider plan adjustment.');
|
||||||
}
|
}
|
||||||
|
|
||||||
const progressPercent = Math.round((summary.completed / summary.total) * 100);
|
const progressPercent = summary.total > 0
|
||||||
|
? Math.round((summary.completed / summary.total) * 100)
|
||||||
|
: 0;
|
||||||
if (progressPercent < 20 && this._loopState.cycleCount > 10) {
|
if (progressPercent < 20 && this._loopState.cycleCount > 10) {
|
||||||
recommendations.push('Progress is slow. Consider simplifying tasks or reviewing dependencies.');
|
recommendations.push('Progress is slow. Consider simplifying tasks or reviewing dependencies.');
|
||||||
}
|
}
|
||||||
|
|
||||||
if (summary.blocked > summary.total / 3) {
|
if (summary.total > 0 && summary.blocked > summary.total / 3) {
|
||||||
recommendations.push('Many tasks are blocked. Review dependency chain for bottlenecks.');
|
recommendations.push('Many tasks are blocked. Review dependency chain for bottlenecks.');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -1043,6 +1043,9 @@ export class RespawnController extends EventEmitter {
|
|||||||
this.log('Stopping respawn controller');
|
this.log('Stopping respawn controller');
|
||||||
this.aiChecker.cancel();
|
this.aiChecker.cancel();
|
||||||
this.planChecker.cancel();
|
this.planChecker.cancel();
|
||||||
|
// Remove event listeners from checkers to prevent memory leaks
|
||||||
|
this.aiChecker.removeAllListeners();
|
||||||
|
this.planChecker.removeAllListeners();
|
||||||
this.clearTimers();
|
this.clearTimers();
|
||||||
this.stopDetectionUpdates();
|
this.stopDetectionUpdates();
|
||||||
this.setState('stopped');
|
this.setState('stopped');
|
||||||
|
|||||||
@@ -717,10 +717,10 @@ export class ScreenManager extends EventEmitter {
|
|||||||
} catch {
|
} catch {
|
||||||
// Fall back to individual queries if batch fails
|
// Fall back to individual queries if batch fails
|
||||||
const statsPromises = screens.map(screen => this.getProcessStats(screen.sessionId));
|
const statsPromises = screens.map(screen => this.getProcessStats(screen.sessionId));
|
||||||
const allStats = await Promise.all(statsPromises);
|
const results = await Promise.allSettled(statsPromises);
|
||||||
return screens.map((screen, i) => ({
|
return screens.map((screen, i) => ({
|
||||||
...screen,
|
...screen,
|
||||||
stats: allStats[i] || undefined
|
stats: results[i].status === 'fulfilled' ? (results[i].value ?? undefined) : undefined
|
||||||
}));
|
}));
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -738,8 +738,13 @@ export class ScreenManager extends EventEmitter {
|
|||||||
}
|
}
|
||||||
|
|
||||||
this.statsInterval = setInterval(async () => {
|
this.statsInterval = setInterval(async () => {
|
||||||
const screensWithStats = await this.getScreensWithStats();
|
try {
|
||||||
this.emit('statsUpdated', screensWithStats);
|
const screensWithStats = await this.getScreensWithStats();
|
||||||
|
this.emit('statsUpdated', screensWithStats);
|
||||||
|
} catch (err) {
|
||||||
|
// Log but don't crash - stats collection is non-critical
|
||||||
|
console.error('[ScreenManager] Stats collection error:', err);
|
||||||
|
}
|
||||||
}, intervalMs);
|
}, intervalMs);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -191,12 +191,19 @@ export class SessionManager extends EventEmitter {
|
|||||||
|
|
||||||
/**
|
/**
|
||||||
* Stops all active sessions.
|
* Stops all active sessions.
|
||||||
|
* Uses Promise.allSettled to ensure all sessions are stopped even if some fail.
|
||||||
*/
|
*/
|
||||||
async stopAllSessions(): Promise<void> {
|
async stopAllSessions(): Promise<void> {
|
||||||
const stopPromises = Array.from(this.sessions.keys()).map((id) =>
|
const stopPromises = Array.from(this.sessions.keys()).map((id) =>
|
||||||
this.stopSession(id)
|
this.stopSession(id)
|
||||||
);
|
);
|
||||||
await Promise.all(stopPromises);
|
const results = await Promise.allSettled(stopPromises);
|
||||||
|
// Log any failures but don't throw - best effort cleanup
|
||||||
|
for (const result of results) {
|
||||||
|
if (result.status === 'rejected') {
|
||||||
|
console.error('[SessionManager] Failed to stop session:', result.reason);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user