Task.Run with async lambda

Question

public class EventService
{
    public async Task InsertEventItemAsync(InsertEventItem request)
    {
        await SomeMethodThatUsesRestSharpToMakeAsynchronousWebApiCall(request);
    }

    public async Task<int> GetNumberOfEvents()
    {
        int result = await SomeOtherMethodThatUsesRestSharpToMakeAsynchronousWebApiCall();
        return result;
    }
}

public interface IFoo
{
    void Bar();
    int Baz();
}

public class Foo : IFoo
{
    EventService _eventService;

    public void Bar()
    {
        Task.Run(async () => await _eventService.InsertEventItemAsync());
    }

    public int Baz()
    {
        int result = Task.Run(async () => await this._eventService.GetNumberOfEvents()).Result;
        return result;
    }
}

That call to Task.Run in Foo.Bar() does not look right to me. It is the approach to async code that is used everywhere in the codebase for a project I have recently started on. My assumption is that that async lambda was written just so that the code will compile. I am doubtful that it a good approach.

I don't have much experience with async/await but I think that this will start a new ThreadPool thread and block the current thread until the task has finished runninng on the ThreadPool thread, which will result is worse performance than if the whole lot was synchronous.

Is that code "wrong"?

It seems to me like there might be an aversion (or possibly a good reason) to going async all the way. Should I try to make the case for going either fully async or fully sync and not trying to mix them?

Thanks.

Foo.Bar will start a new task that executes on the task pool. No new thread is created. Foo.Bar returns immediately while the task executes asynchronously ("fire and forget"). Is the code wrong? It depends on what you want to achieve. You can replace the async delegate with a "normal" delegate for a slight optimization in the generated IL. — Martin Liversage
– Martin Liversage, Commented Aug 11, 2017 at 8:56
I've updated the question to show how calls are made to methods that return Task<T>. If Task.Run is fire and forget, does that mean that calling Task.Run(...).Result prevents Foo.Baz from returning immediately and instead forces it to wait until the task has completed? Does that increase the likelihood of deadlocks? — dlarkin77
– dlarkin77, Commented Aug 11, 2017 at 9:12
Getting the Result property of a task will block until the task completes so it no longer is "fire and forget". The likelihood of deadlocks in your code should be zero. If it is not you need to fix the root cause of the deadlocks. However, when your code involves callbacks into code that is not reentrant you can sometimes avoid deadlocks by scheduling these callbacks on new tasks/threads. Your questions seem to be about a general understanding of async and await and I suggest that you study this topic and/or write some simple code to gain a better overall understanding. — Martin Liversage
– Martin Liversage, Commented Aug 11, 2017 at 9:30

Stephen Cleary · Accepted Answer · 2017-08-11 15:06:08Z

That call to Task.Run in Foo.Bar() does not look right to me.

Yes, it is Very Bad™.

Task.Run executes its delegate on a thread pool thread. Since it's an asynchronous delegate, it won't "use up" a thread pool thread any time it is "awaiting" - so you don't need to worry about that.

What you do need to worry about is what happens to the task returned from Task.Run. In particular, nothing is done with it. This is problematic because if the delegate throws an exception, that exception is placed on the task returned from Task.Run, which is... completely ignored. So you have silent failures. This is exactly as bad as writing catch { } all over the place.

Foo.Baz is using the thread pool hack (unfortunately using Result, which makes for more awkward error handling). This hack is a way of writing sync-over-async code that has no possibility of deadlocks.

It seems to me like there might be an aversion (or possibly a good reason) to going async all the way. Should I try to make the case for going either fully async or fully sync and not trying to mix them?

Yes. Ideally, the code should be async all the way (or sync all the way). There are a few situations where this is infeasible, but the vast majority of code should certainly be one or the other. Hacks like this are acceptable during a transition time when legacy code is being updated to async, but they shouldn't exist in an application long-term without good reason.

Collectives™ on Stack Overflow

Task.Run with async lambda

1 Answer 1

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Related