danielgerlag / workflow-core

Lightweight workflow engine for .NET Standard
MIT License
5.38k stars 1.2k forks source link

StopAsync does not wait for tasks completion #645

Open ssougnez opened 4 years ago

ssougnez commented 4 years ago

Hi,

I created a custom step where I put this:

await Task.Delay(60000);

Then, I start a workflow with this step and I call an API route that does this:

await _wfHost.StopAsync(new CancellationToken());

And this call returns immediately, not waiting for the Task.Delay to finish. Is it a normal behavior? I expected StopAsync to wait for the delay to finish, otherwise, it means that StopAsync is not graceful.

Cheers

danielgerlag commented 4 years ago

That sounds like a problem. Will look into it this weekend.

danielgerlag commented 4 years ago

Try with v3.2.4 and let me know if that helps?

danielgerlag commented 4 years ago

There seems to be something weird with detached child tasks, where the parent task has status RanToCompletion before it's really finished. Try this example.

        static void  Main(string[] args)
        {
            var task = new Task(async () =>
            {

                Console.WriteLine("Hello World!");

                await Task.Delay(3000);

                Console.WriteLine("Done!");
            });

            task.Start();
            task.Wait();

            Console.WriteLine("END");
            Console.ReadLine();
        }
ssougnez commented 4 years ago

Hi,

as you can expect, the 3.2.4 did not solve this issue. I tried your sample and indeed, the behavior seems weird. On the other hand, I guess there must be some kind of explanation because it would be weird that it's a bug. I'll try searching Google when I have time to see if there's an explanation to this.

If you find one, I'm interested ;-)

Edit 1

Note that I tried this and it works as expected:

static void Main(string[] args)
{
    var task = Task.Run(async () =>
    {

        Console.WriteLine("Hello World!");

        await Task.Delay(3000);

        Console.WriteLine("Done!");
    });

    //task.Start();
    task.Wait();

    Console.WriteLine("END");
    Console.ReadLine();
}

Edit 2

Here is a nice SO post talking about that: https://stackoverflow.com/questions/56441486/task-wait-doesnt-wait-for-async-method-completion/56442081

Looks like Task.Run is the way to go and Task constructor should never be used (more info: https://blog.stephencleary.com/2014/05/a-tour-of-task-part-1-constructors.html).

danielgerlag commented 4 years ago

Yeah, the problem is that Task.Run doesn't have any overloads for passing parameters to the task.

danielgerlag commented 4 years ago

@ssougnez give 3.2.5 a spin and let me know?

ssougnez commented 4 years ago

Didn't work for me, sorry

danielgerlag commented 4 years ago

That's wierd, I tested the example you gave above and it worked. Was that a proxy for the real code?

ssougnez commented 4 years ago

Well, the application is pretty big so yeah it's a simplification of the code but basically, I added a task delay in a custom action than call a rest API to shutdown the host.

I did the test in a hurry maybe I missed something. I'll try again tomorrow and tells you what.

ssougnez commented 4 years ago

I tested and I confirm that it still does not work for me. I can't give you all the code as it's a full blown application but I'll try to give the main parts.

I have a class BaseStepAsync that inherits from StepBodyAsync. Basically, it is just to be able to perform some instructions before and after the real instructions of the steps. Indeed, I have to check in the database if a flag is set to true and do some logging, so basically, here is the simplified code of this step:

namespace Foo
{
    public abstract class BaseStepAsync : StepBodyAsync
    {
        public abstract Task<ExecutionResult> ExecuteAsync(IStepExecutionContext context);

        public override async Task<ExecutionResult> RunAsync(IStepExecutionContext context)
        {
            // Some steps

            var result = await ExecuteAsync(context);

            if (result.Proceed == false)
            {
                return result;
            }

            // Some other steps

            return result;
        }
    }
}

Now, custom steps can simply inherit from this class and override ExecuteAsync. I modified one of these steps like this:

public override async Task<ExecutionResult> ExecuteAsync(IStepExecutionContext context)
{
    await LogInfoAsync("Registering the event");

    await Task.Delay(60000);

    // Content of the action

    return ExecutionResult.Next();
}

If I test it, it works, the workflow waits for 1 min before doing the step instructions. On the same application, I also have a REST API that triggers the following method:

public async Task<bool> StartMaintenanceAsync()
{
    var scheduler = await _schedulerFactory.GetScheduler();

    await scheduler.Shutdown(true);

    await _wfHost.StopAsync(new CancellationToken());

    return true;
}

Basically, I'm stopping the Quartz scheduler, then I stop the workflow host. This one is just injected in the constructor as IWorkflowHost.

My test is the following:

Cheers

danielgerlag commented 4 years ago

are you able to encapsulate this into a failing test? I have not been able to reproduce it.

hol353 commented 3 years ago

I'm also seeing this behaviour. I'm new to WorkflowCore so might be doing something wrong.

I'm using version 3.3.5. My console app is targetting .Net Core 3.1. My code is below. I expected the code below to write 'Workflow completed' and then 'Program completed'. It only writes 'Program completed' indicating the call to host.StopAsync(CancellationToken.None).Wait(); does not wait for the workflow to finish. Suggestions?

using Microsoft.Extensions.DependencyInjection;
using System;
using System.Threading;
using System.Threading.Tasks;
using WorkflowCore.Interface;
using WorkflowCore.Models;

namespace CloudRunner
{
    class Program
    {
        static void Main(string[] args)
        {
            IServiceProvider serviceProvider = ConfigureServices();

            //start the workflow host
            var host = serviceProvider.GetService<IWorkflowHost>();
            host.RegisterWorkflow<TestWorkflow>();
            host.Start();

            var t = host.StartWorkflow("HelloWorld");

            //Console.Read();

            // Shouldn't this line wait for workflow to complete?
            host.StopAsync(CancellationToken.None).Wait();

            Console.WriteLine("Program completed");
        }

        private static IServiceProvider ConfigureServices()
        {
            // setup dependency injection
            IServiceCollection services = new ServiceCollection();
            services.AddLogging();
            services.AddWorkflow();
            //services.AddWorkflow(x => x.UseMongoDB(@"mongodb://localhost:27017", "workflow"));

            var serviceProvider = services.BuildServiceProvider();

            return serviceProvider;
        }
    }

    public class TestWorkflow : IWorkflow
    {
        public void Build(IWorkflowBuilder<object> builder)
        {
            builder.StartWith(context =>
            {
                Task.Delay(60000);
                return ExecutionResult.Next();
            })
            .Then(context =>
            {
                Console.WriteLine("Workflow completed");
                return ExecutionResult.Next();
            });
        }

        public string Id => "HelloWorld";

        public int Version => 1;
    }
}