c# - Calling an async method using a Task.Run seems wrong?

Question

I recently came across this code written by a contractor we had working for us. It's either devilishly clever or silly (I think the latter but I wanted a second opinion). I'm not massively up to speed on async await.

Basically it worked like this:

public bool Send(TemplatedMessageDto message)
{
    return Task.Run(() => SendAsync(message))
        .GetAwaiter()
        .GetResult();
}

public async Task<bool> SendAsync(TemplatedMessageDto message)
{
    //code doing stuff
    var results = await _externalresource.DothingsExternally();
    //code doing stuff
}

Now as I understand it that first Task.Run() is pointless and inefficient? and should really be:

public bool Send(TemplatedMessageDto message)
{
    return SendAsync(message))
    .GetAwaiter()
    .GetResult();
}

public async Task<bool> SendAsync(TemplatedMessageDto message)
{
    //code doing stuff
    var results = await _externalresource.DothingsExternally();
    //code doing stuff
}

I'm also not convinced this is really an async method because it will still wait, right? I think it's only advantage (even re-written) is to free up the main worker thread.

Can someone confirm that this first Task shouldn't be there?

Answer 1

I'm also not convinced this is really an async method because it will still wait, right?

It isn't, as Yuval explained. You shouldn't use sync over async.

Now as I understand it that first Task.Run() is pointless and inefficient?

Not really, there is value in using Task.Run in such a way.

Since you're blocking on an async method (which you shouldn't do) there is a chance you'll deadlock. That happens in UI apps and asp.net where you have a SynchronizationContext.

Using Task.Run clears that SynchronizationContext since it offloads the work to a ThreadPool thread and removes the risk for a deadlock.

So, blocking is bad, but if you end up doing it using Task.Run is safer.