fix: prevent suspend/resume event buffer infinite loop#306
Open
YunchuWang wants to merge 1 commit into
Open
Conversation
handleExecutionResumed iterates _suspendedEvents using for..of while processEvent can push new entries onto the same array. If a buffered ExecutionSuspended event sets _isSuspended=true during iteration, subsequent suspendable events are re-pushed to the array being iterated, creating an infinite loop that hangs the worker process. Two fixes: 1. Snapshot _suspendedEvents before iterating and clear it first. Events buffered during processing land in the new (empty) array and are correctly held for the next Resume event. 2. Add ExecutionSuspended to the isSuspendable exclusion list so suspend events are processed immediately rather than buffered. Also uses strict equality (===) instead of loose (==) for the indexOf check. Co-authored-by: Copilot <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a worker hang in the Durable Task JS orchestration executor by preventing re-entrant modification of the suspended event buffer during resume processing, and adds regression tests to ensure suspend/resume buffering remains safe.
Changes:
- Snapshot-and-clear
_suspendedEventsbefore replaying buffered events onExecutionResumedto prevent an infinite loop. - Treat
ExecutionSuspendedas non-suspendable (never buffered) and tighten the suspendable check to strict equality. - Add regression tests covering buffered events that include an extra suspend and ensuring
ExecutionSuspendedis never buffered.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/durabletask-js/src/worker/orchestration-executor.ts | Snapshots/clears suspended event buffer before iterating to avoid infinite loop on resume. |
| packages/durabletask-js/src/worker/index.ts | Updates isSuspendable to exclude ExecutionSuspended and use strict equality. |
| packages/durabletask-js/test/orchestration_executor.spec.ts | Adds regression tests for suspend/resume buffering edge cases and infinite-loop prevention. |
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.
Summary
Fixes #202
Testing
npm test -w @microsoft/durabletask-js -- --runTestsByPath test/orchestration_executor.spec.tsnpm run build:core