danielgerlag / danielgerlag/workflow-core
`WorkflowHost.Stop()` does not shut down the host gracefully, may lose persistence
- Dominant language
- C#
- Stars
- 5.9k
- Forks
- 1.3k
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 5
Description
**Describe the bug**
When shutting down the host with `WorkflowHost.Stop()`/`.StopAsync()`, the `CancellationToken` supplied to all operations is cancelled immediately. This can lead to the state of just completed workflows/steps not being persisted in time before being cancelled themselves.
**To Reproduce**
Stop the host on `LifeCycleEvent` `WorkflowCompleted`, e.g.:
```csharp
[Fact]
public async Task Scenario()
{
var tcs = new TaskCompletionSource();
Host.OnLifeCycleEvent += (evt) => OnLifeCycleEvent(evt, tcs);
var workflowId = StartWorkflow(null);
await tcs.Task;
GetStatus(workflowId).Should().Be(WorkflowStatus.Complete);
}
private async void OnLifeCycleEvent(LifeCycleEvent evt, TaskCompletionSource tcs)
{
if (evt is WorkflowCompleted)
{
await Host.StopAsync(CancellationToken.None);
tcs.SetResult(new());
}
}
```
**Expected behavior**
The workflow's `Completed` state is persisted. But for several persistence providers it is not, because `IPersistenceProvider.PersistWorkflow()` is cancelled [here](https://github.com/danielgerlag/workflow-core/blob/70e510421b6d69d4d37eacd04404ac06ad4f9cb2/src/WorkflowCore/Services/BackgroundTasks/WorkflowConsumer.cs#L58).
**Additional context**
I noticed this for the concrete example above but could imagine that several other persistence operations are affected as well. Generally, I would question if persistence operations should be cancellable at all.
I have created some tests to reproduce the issue here: https://github.com/mamidenn/workflow-core/blob/fix-race-condition-on-stop/test/WorkflowCore.IntegrationTests/Scenarios/StopScenario.cs
I will gladly open a PR to fix this issue but would like to get your feedback on what kind of solution you would prefer. I can generally think of
* not passing the `WorkflowHost`'s `CancellationToken` to the persistence operations
* Removing the `CancellationToken` parameter from all write operations in `IPersistenceProvider`
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with WorkflowCore/Services/BackgroundTasks/WorkflowConsumer.cs at the cancellation point referenced in the issue, then trace WorkflowHost.Stop() and StopAsync(). Run or inspect test/WorkflowCore.IntegrationTests/Scenarios/StopScenario.cs to reproduce the race. Done means the completed workflow state is persisted when the host stops, with related persistence operations covered by tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100