fix: Cancel pending timer handles when purging orchestration instances#197
Open
YunchuWang wants to merge 1 commit intomainfrom
Open
fix: Cancel pending timer handles when purging orchestration instances#197YunchuWang wants to merge 1 commit intomainfrom
YunchuWang wants to merge 1 commit intomainfrom
Conversation
The InMemoryOrchestrationBackend.purge() method deleted orchestration instances from the store but did not cancel their pending setTimeout handles. This caused timer handles to leak, keeping the Node.js event loop alive unnecessarily and wasting resources until the timers eventually fired and found no instance to act on. The fix adds per-instance timer tracking via an instanceTimers map. When purge() is called, all pending timers for that instance are cancelled and removed from both the instance-level and global timer tracking sets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a resource-leak in the InMemoryOrchestrationBackend test backend by ensuring durable timer setTimeout handles are canceled when a terminal orchestration instance is purged, preventing leaked timers from keeping the Node.js event loop alive (e.g., Jest “did not exit” warnings).
Changes:
- Add per-instance timer tracking (
instanceTimers) alongside the existing globalpendingTimers. - Cancel and deregister all timers associated with an instance inside
purge(). - Add Jest coverage to validate timers are canceled for the purged instance only.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/durabletask-js/src/testing/in-memory-backend.ts | Track timer handles per orchestration instance and cancel them during purge; keep reset behavior clearing all timers. |
| packages/durabletask-js/test/in-memory-backend.spec.ts | Add tests asserting purge cancels pending timers and doesn’t affect other instances’ timers. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #188
Problem
InMemoryOrchestrationBackend.purge() deletes the orchestration instance from the store but does not cancel pending setTimeout handles created by processCreateTimerAction(). Timer handles leak, keeping the Node.js event loop alive unnecessarily (can cause Jest 'did not exit' warnings).
Changes