-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Description
Problem
As identified in PR #136 code review, the polling setTimeout in waitForAvailableWorker continues recursively after the timeout rejects, potentially leaking timers.
Current Behavior
private async waitForAvailableWorker(timeoutMs: number = 5000): Promise<WorkerProcess> {
return new Promise((resolvePromise, reject) => {
const timeout = setTimeout(() => {
reject(new Error('Timeout waiting for available worker'));
}, timeoutMs);
const checkWorker = (): void => {
const worker = this.selectOptimalWorker();
if (worker) {
clearTimeout(timeout);
resolvePromise(worker);
} else {
setTimeout(checkWorker, 10); // ← Continues after timeout rejects
}
};
checkWorker();
});
}Proposed Fix
Track the polling timer and clear it on timeout:
private async waitForAvailableWorker(timeoutMs: number = 5000): Promise<WorkerProcess> {
return new Promise((resolvePromise, reject) => {
let pollTimer: NodeJS.Timeout | null = null;
let resolved = false;
const timeout = setTimeout(() => {
resolved = true;
if (pollTimer) clearTimeout(pollTimer);
reject(new Error('Timeout waiting for available worker'));
}, timeoutMs);
const checkWorker = (): void => {
if (resolved) return;
const worker = this.selectOptimalWorker();
if (worker) {
resolved = true;
clearTimeout(timeout);
resolvePromise(worker);
} else {
pollTimer = setTimeout(checkWorker, 10);
}
};
checkWorker();
});
}Impact
Low - The leaked timers resolve to no-ops once the Promise settles (resolved promises ignore subsequent resolve/reject calls). However, this is a code quality issue that should be fixed.
References
- PR refactor: unify NodeBridge and OptimizedNodeBridge (ADR-0001) #136 CodeRabbit review comment
- Related to ADR-0001 bridge unification
Metadata
Metadata
Assignees
Labels
No labels