-
Notifications
You must be signed in to change notification settings - Fork 10
Open
Labels
copilot-findsFindings from daily automated code review agentFindings from daily automated code review agent
Description
Problem
In InMemoryOrchestrationBackend.waitForState() (in-memory-backend.ts), the timeout handler attempts to clean up the stale waiter by comparing w.resolve === resolve, but this comparison always fails because w.resolve is a wrapper function — not the original Promise resolve.
Code (before fix):
const timer = setTimeout(() => {
const waiters = this.stateWaiters.get(instanceId);
if (waiters) {
const index = waiters.findIndex((w) => w.resolve === resolve);
// ^^^^^^^^^^^^^^^^^^^^^^^^
// BUG: w.resolve is a wrapper (result) => { clearTimeout(timer); resolve(result); }
// so it will never === resolve. findIndex always returns -1.
if (index >= 0) {
waiters.splice(index, 1); // Never executed
}
}
reject(new Error(...));
}, timeoutMs);
const waiter: StateWaiter = {
resolve: (result) => {
clearTimeout(timer);
resolve(result); // ← wraps resolve, so waiter.resolve !== resolve
},
...
};Root Cause
The StateWaiter.resolve property is set to a wrapper function that calls clearTimeout(timer) before calling the original Promise resolve. The timeout handler then tries to find the waiter by comparing w.resolve === resolve — but w.resolve is the wrapper, not the original resolve, so the identity check always fails.
Impact
- Memory leak: Timed-out waiters permanently accumulate in the
stateWaitersmap. - Wasted computation:
notifyWaiters()continues evaluating predicates on stale waiters. - Timer leak: The
waitForStatetimeout timer is not tracked inpendingTimers, so it is not cleaned up byreset(). - Affects any test using
waitForState(indirectly viaTestOrchestrationClient.waitForOrchestrationCompletion,waitForOrchestrationStart, etc.) where the wait times out. - No data corruption since calling
resolve()on a settled Promise is a no-op, but the stale waiter and its closure remain in memory.
Proposed Fix
- Move the
waiterdeclaration before thetimerso the timeout callback can usewaiters.indexOf(waiter)(object identity) instead offindIndexwith a broken reference comparison. - Track the
waitForStatetimeout timer inpendingTimerssoreset()cleans it up. - Remove the timer from
pendingTimerswhen the waiter resolves, rejects, or times out. - Clean up the
stateWaitersmap entry when the last waiter for an instance is removed.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
copilot-findsFindings from daily automated code review agentFindings from daily automated code review agent