Async support

[mbp]

Most of the time when I'm implementing jobs, I have some async involved. However since this code is executed from an IJob, I usually have to write code like:

public class Job : IJob
{
    public void Execute()
    {
        MethodAsync().Wait();
    }
}

It would be nice if the library had support for an IJob of the async variant, so I could instead write:

public class Job : IJob
{
    public async Task Execute()
    {
        await MethodAsync();
    }
}

[phillip-haydon]

That already works??? I'm using it right now...

[mbp]

No it doesn't. IJob requires you to implement a method void Execute(). For async to work, it should return Task.

[phillip-haydon]

Making it void only means it would become fire and forget, so NonReentrant will not work with it.

So it does work, but only if you don't mind not having NonReentrant for now.

[mbp]

Making it public async void Execute() has big problems. As you mention, it becomes fire and forget. So then FluentScheduler won't actually know when the job finished, and it will call Dispose() on the job immediately efter calling Execute(). In this case it should obviously await the job.

[phillip-haydon]

Hmmm I had 2 jobs running on my laptop all night using public async void Execute() with async calls to YouTube api and async calls to insert into the database. it didn't dispose of the job class immediately... The jobs take ~2m to run with a new job spinning up every 5 minutes.

Didn't run into any issues. Other than noticing NonReentrant is pretty broken.

[mbp]

Most likely you don't use IDisposable on your jobs, so you didn't experience the problem.

Here is example code to show case the issue with using public async void Execute():

public class JobRegistry : Registry
{
    public JobRegistry()
    {
        Schedule<Job>().ToRunEvery(1).Seconds();
    }
}

public class Job : IJob, IDisposable
{
    public static int JobNumber;

    private readonly int _job;

    public Job()
    {
        _job = JobNumber++;
    }

    public async void Execute()
    {
        Console.WriteLine($"Async job {_job} started");
        await Task.Delay(2000);
        Console.WriteLine($"Async job {_job} still running");
        await Task.FromResult(0);
    }

    public void Dispose()
    {
        Console.WriteLine($"Disposing job {_job}");
    }
}

Output:

Async job 0 started
Disposing job 0
Async job 1 started
Disposing job 1
Async job 0 still running

So both this example and your NonReentrant example proves that you cannot do "native" async with FluentScheduler, and that would be very nice :-)

[phillip-haydon]

Okie dokie.

Yeah native support is better, but it is possible to get basic* support now.

[tallesl]

If you really want to ditch the Wait() call for the await sugary, why not do it yourself?

public class Job : IJob
{
    public void Execute()
    {
        ExecuteAsync().Wait();
    }

    private async Task ExecuteAsync()
    {
        // do your work
    }
}

As for NonReentrant being broken, I'll tackle it on (#107).

[mbp]

That's what I am doing now.

But like I wrote in first post:

It would be nice if the library had support for an IJob of the async variant,

Simply because I don't feel that I should repeating this pattern all the time. More and more C# code is getting async, so why not have support for it in the library? It totally make sense to me that a library that is supposed to execute my program logic, should have support for doing so async.

Not sure why this was closed.

[tallesl]

The only way to make such support possible, as I see it, it's by creating a IAsyncJob or at least an abstract AsyncJob, but I see little value in doing so. There would be some impact in the current code (although it wouldn't be much) plus the possible confusion of having two 'kinds' of job behaving practically equal for the scheduler.

If you're feeling that you're repeating this pattern all the time within a codebase, may I suggest you to create something like this:

public abstract class AsyncJob : IJob
{
    public void Execute()
    {
        ExecuteAsync().Wait();
    }

    protected abstract Task ExecuteAsync();
}

[delixfe]

Hi @tallesl

I am currently fixing code using FluentScheduler. The authors did not recognize the current async behavior thus jobs were started non reentrant causing some hard to understand issues. I think the prefix "Async" is used in many places where new async supporting methods are created next to the existing ones.

What about having an IAsyncJob interface and additionally ScheduleAsync methods on the JobManager and JobRegistry?

I do not think this solution would add confusion as this pattern is used widely. Having the ...Async methods and interface should get people thinking at least.

[tallesl]

DoSomething runs the method synchronously (blocks whoever called it) and DoSomethingAsync runs asynchronously (doesn't block). One gives you a value, and the other a future for the value (Task).

That pattern makes sense.

Now, Schedule and ScheduleAsync would be extremely confusing. Both would be called asynchronously by the library. FluentScheduler only needs a synchronous Execute and nothing else.

If you, for whatever reason, need the async/await sugar, well, you do it. See my couple of comments above.

[sharok]

@tallesl The another reason to have the async Execute is cathcing Exception. I have job that call async method which make request to 3-rd party API and returns some data. And, if I use .Result and there is some error, then there will be AggregateException and inside there will be HttpException (InnerException). await helps to handle it correcly. So, in the current implementation I always have to go InnerException in order to get actual error. The AsyncJob class that you provided above solve it, but I think this feature should be out of the box.

[fubar-coder]

@tallesl Please reconsider your opinion about async/await support. The problem is, that using .Wait() causes deadlocks in WPF (not sure about Windows Forms) applications when using HttpClient in this async function.

[AlonAm]

Considering this code

public void Execute()
{
    ExecuteAsync().Wait();
}

What happens when the job is performing an IO operation within the async task? Is it releasing the task until IO is completed?

[bonesoul]

any updates on this?

[VitorCioletti]

There are no plans on developing new features in the current state of the library .See #214.

[tallesl]

Due to some C# 8 shenanigans, I just added a IAsyncJob on version 5.5.0.

It's more of a dirty hack to be honest:

void IJob.Execute() 
{
    ExecuteAsync().Wait();
}

It's especially difficult to get rid of Wait()on version 5 since there's another one at the very core of the scheduling, and I'm not messing with that.

Things should be much better on the upcoming version 6 (#256).

[AlonAm]

Shouldn't you be using .GetAwaiter().GetResult() instead?

[tallesl]

Edit: see #280

[AlonAm]

.GetAwaiter().GetResult() will produce a nicer stack trace when exceptions occur