danielgerlag / danielgerlag/workflow-core

`WorkflowHost.Stop()` does not shut down the host gracefully, may lose persistence

Open
#1,032 4 comments 3 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.