Isolate background workflow dispatches from caller's cancellation token#7291
Isolate background workflow dispatches from caller's cancellation token#7291avin3sh wants to merge 1 commit intoelsa-workflows:mainfrom
Conversation
When dispatching multiple child workflows, early child completions cancel the parent activity's token while dispatches are still in progress. This creates a race condition where remaining dispatches fail with OperationCanceledException.
Greptile OverviewGreptile SummaryFixes race condition where child workflow dispatches fail when early-completing children cancel the parent's token while dispatches are still pending. Changed all four dispatch methods in
The fix is particularly important for Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| src/modules/Elsa.Workflows.Runtime/Services/BackgroundWorkflowDispatcher.cs | Changes all four dispatch methods to use CancellationToken.None instead of caller's token for background command execution, preventing race conditions when child workflows complete early |
| test/unit/Elsa.Workflows.Runtime.UnitTests/Services/BackgroundWorkflowDispatcherTests.cs | New test file with four comprehensive tests verifying that each dispatch method correctly isolates background commands from caller's cancellation token |
Sequence Diagram
sequenceDiagram
participant Caller
participant Dispatcher as BackgroundWorkflowDispatcher
participant NotificationSender
participant CommandSender
participant BackgroundQueue as Background Command Queue
Caller->>Dispatcher: DispatchAsync(request, callerToken)
Dispatcher->>NotificationSender: SendAsync(Dispatching notification, callerToken)
NotificationSender-->>Dispatcher:
Dispatcher->>CommandSender: SendAsync(command, Background, headers, CancellationToken.None)
Note over CommandSender,BackgroundQueue: Command queued with independent token
CommandSender-->>Dispatcher:
Dispatcher->>NotificationSender: SendAsync(Dispatched notification, callerToken)
NotificationSender-->>Dispatcher:
Dispatcher-->>Caller: Success response
Note over Caller: Caller may cancel/complete here
Note over BackgroundQueue: Background command continues<br/>execution independently
Last reviewed commit: 4204fc1
|
|
||
| await commandSender.SendAsync(command, CommandStrategy.Background, CreateHeaders(), cancellationToken); | ||
| // Background commands run independently of caller's lifecycle. | ||
| await commandSender.SendAsync(command, CommandStrategy.Background, CreateHeaders(), CancellationToken.None); |
There was a problem hiding this comment.
Consider applying this same pattern to BackgroundStimulusDispatcher and BackgroundWorkflowCancellationDispatcher for consistency, as they also dispatch background commands that should run independently of caller lifecycle.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Pull request overview
This PR prevents background workflow dispatches from being canceled by the caller’s CancellationToken, addressing a race where early-finishing child workflows (notably via BulkDispatchWorkflows) can cancel the parent token while dispatches are still being enqueued.
Changes:
- Use
CancellationToken.Nonewhen sending workflow dispatch commands withCommandStrategy.Background. - Add unit tests verifying caller tokens are not propagated to background command dispatch.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/modules/Elsa.Workflows.Runtime/Services/BackgroundWorkflowDispatcher.cs |
Ensures background dispatch commands are enqueued independently from the caller’s cancellation token. |
test/unit/Elsa.Workflows.Runtime.UnitTests/Services/BackgroundWorkflowDispatcherTests.cs |
Adds coverage to assert CancellationToken.None is used for background command sends across dispatcher overloads. |
Child workflow dispatches fail when early-completing child workflows cancel the parent's cancellation token while dispatches are still in progress. This race condition is most visible with
BulkDispatchWorkflowsas it has potential to create a situation where with enough "items", some children complete while dispatches are still pending, but, technically, this bug affects all dispatch scenarios.The solution here is to use
CancellationToken.Nonefor background commands - they should run independently:This pattern already exists in the Bookmark Resume endpoint for the same reason,
elsa-core/src/modules/Elsa.Workflows.Api/Endpoints/Bookmarks/Resume/Endpoint.cs
Lines 41 to 45 in c60e63c
Note: this fix complements #7218
This fix complements the separate
CommandHandlerInvokerMiddlewarefix:OperationCanceledException(not wrapped inTargetInvocationException) otherwise it breaks the task processing inBackgroundCommandSenderHostedServicedue to unhandled exception