danielgerlag / danielgerlag/workflow-core
WorkflowTermination not ending the workflow consistently
- Dominant language
- C#
- Stars
- 5.9k
- Forks
- 1.3k
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 5
Description
**Describe the bug**
After calling await workflowHost.TerminateWorkflow(workflow.Id) the workflow does not terminate as expected and will return False to confirm that the termination did not occur.
**To Reproduce**
Steps to reproduce the behaviour:
1. Start a workflow.
2. Retrieve the workflow ID.
3. Call await workflowHost.TerminateWorkflow(workflow.Id).
4. The return value is likely to be False, and the workflow will not have terminated.
(Please note the test code below to reproduce this)
**Expected behaviour**
I expect:
1. That the currently executing step will have it's cancellation token set.
2. That the workflow will terminate.
3. That the final status of the workflow will be Terminated.
**Additional context**
As mentioned in the 2nd related issue below, this is because:
"_If the workflow is running then it returns false and does nothing.
That happens because the implementation first checks if there is a lock associated with the given workflow and if so it immediately returns false and does nothing.
Since there is always a lock present for a running workflow, effectively it means that we can terminate only non-running workflows which is very unfortunate._"
**Related Issues**
Also reported in two other issues:
1. https://github.com/danielgerlag/workflow-core/issues/1170
2. https://github.com/danielgerlag/workflow-core/issues/973
The workarounds suggested are not the right answers.
**Test Code Scenario to Reproduce the issue**
```
using FluentAssertions;
using System;
using System.Threading;
using WorkflowCore.Interface;
using WorkflowCore.Models;
using WorkflowCore.Testing;
using Xunit;
namespace WorkflowCore.IntegrationTests.Scenarios
{
public class MyInitialStep() : StepBody
{
public static TerminationThread Thread { get; set; }
public MyDataClass MyData { get; set; }
public override ExecutionResult Run(IStepExecutionContext context)
{
Console.WriteLine("MyInitialStep: In MyInitialStep, starting termination thread");
Thread = new TerminationThread(MyData);
return ExecutionResult.Next();
}
}
public class MyStep : StepBody
{
public MyDataClass MyData { get; set; }
public override ExecutionResult Run(IStepExecutionContext context)
{
MyData.WaitEvent1.Set();
Console.WriteLine("MyStep: WaitEvent1 was set, waiting for event 2");
Console.WriteLine($"MyStep: Pre : IsCancellationRequested: {context.CancellationToken.IsCancellationRequested}");
var waitForEvent2 = MyData.WaitEvent2.WaitOne(5000);
Console.WriteLine($"MyStep: Post: IsCancellationRequested: {context.CancellationToken.IsCancellationRequested}");
Console.WriteLine($"MyStep: waitForEvent2: {waitForEvent2}");
MyData.StepWasCancelled = context.CancellationToken.IsCancellationRequested;
context.CancellationToken.ThrowIfCancellationRequested();
return ExecutionResult.Next();
}
}
public class TerminationThread
{
private Thread _thread;
public MyDataClass MyData { get; set; }
public TerminationThread(MyDataClass myDataClass)
{
MyData = myDataClass;
Console.WriteLine("TerminationThread: Starting termination thread");
_thread = new Thread(new ThreadStart(Run));
_thread.Start();
}
public void Run()
{
Console.WriteLine("TerminationThread: Termination thread running");
var waited = MyData.WaitEvent1.WaitOne(5000);
Console.WriteLine($"TerminationThread: Wait Event 1 is {waited}, terminating workflow");
Console.WriteLine("TerminationThread: Terminating workflow");
MyData.TerminatationWasSuccessful = MyData.WorkflowHost.TerminateWorkflow(MyData.WorkflowInstanceId).GetAwaiter().GetResult();
//MyData.WorkflowHost.Stop();
Console.WriteLine($"TerminationThread: TerminateWorkflow returned {MyData.TerminatationWasSuccessful}");
MyData.WaitEvent2.Set();
}
}
public class MyDataClass
{
public EventWaitHandle WaitEvent1 = new EventWaitHandle(false, EventResetMode.ManualReset);
public EventWaitHandle WaitEvent2 = new EventWaitHandle(false, EventResetMode.ManualReset);
public IWorkflowHost WorkflowHost { get; set; }
public string WorkflowInstanceId { get; set; }
public bool TerminatationWasSuccessful { get; set; } = false;
public bool StepWasCancelled { get; set; } = false;
}
public class TerminateWorkflowScenario : WorkflowTest
{
public class EventWorkflow : IWorkflow
{
public string Id => "TerminateWorkflow";
public int Version => 1;
public void Build(IWorkflowBuilder builder)
{
builder
.StartWith().Input(step => step.MyData, data => data)
.Then().Input(step => step.MyData, data => data);
}
}
public TerminateWorkflowScenario()
{
Setup();
}
[Fact]
public void terminate_during_step_should_signal_cancellation_object()
{
// Start a simple workflow and trigger a TerminateWorkflow() call from a separate thread while MyStep is executing.
Console.WriteLine("Starting workflow");
var myData = new MyDataClass()
{
WorkflowHost = Host
};
var workflowId = StartWorkflow(myData);
myData.WorkflowInstanceId = workflowId;
WaitForWorkflowToComplete(workflowId, TimeSpan.FromSeconds(20));
// Check to see if MyStep observed the cancellation request and that the final status is Terminated.
myData.WaitEvent2.WaitOne(5000);
myData.WaitEvent1.WaitOne(1).Should().BeTrue("WaitEvent1 should be set to indicate that MyStep was executed");
myData.TerminatationWasSuccessful.Should().BeTrue("TerminateWorkflow() returned False");
GetStatus(workflowId).Should().Be(WorkflowStatus.Terminated);
myData.StepWasCancelled.Should().BeTrue("Step was not cancelled");
}
}
}
```
**Code changes to Workflow Core that cause the effect that I'm after**
The first 2 lines in the function have been added.
```WorkflowHost.cs
public Task TerminateWorkflow(string workflowId)
{
var workflowConsumer = _backgroundTasks.First(_backgroundTasks => _backgroundTasks is WorkflowConsumer);
workflowConsumer?.Stop();
return _workflowController.TerminateWorkflow(workflowId);
}
```
The call to AcquireLock is removed to allow a running workflow to be cancelled.
```WorkflowController.cs
public async Task TerminateWorkflow(string workflowId)
{
var wf = await _persistenceStore.GetWorkflowInstance(workflowId);
wf.Status = WorkflowStatus.Terminated;
wf.CompleteTime = _dateTimeProvider.UtcNow;
await _persistenceStore.PersistWorkflow(wf);
await _queueProvider.QueueWork(workflowId, QueueType.Index);
await _eventHub.PublishNotification(new WorkflowTerminated
{
EventTimeUtc = _dateTimeProvider.UtcNow,
Reference = wf.Reference,
WorkflowInstanceId = wf.Id,
WorkflowDefinitionId = wf.WorkflowDefinitionId,
Version = wf.Version
});
return true;
}
```
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with the terminate_during_step_should_signal_cancellation_object scenario in the issue and run it to reproduce the failure. Then read TerminateWorkflow in WorkflowHost.cs and WorkflowController.cs, along with the workflow consumer and persistence interactions they call. Done means the scenario passes: termination returns true, the step observes cancellation, and the final status is Terminated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100