1

Consider this code:

public async Task DoStuffAsync() {
    await WhateverAsync(); // Stuff that might take a while
}

// Elsewhere in my project...
public async Task MyMethodAsync() {
    await DoStuffAsync();
}

public void MyMethod() {
    DoStuffAsync(); // <-- I want this to block until Whatever() is complete.
}

I want to provide two methods with the same functionality: an async method which runs asynchronously, and a non-async method that blocks, and either of these two methods can be called depending on whether the code I'm calling from is also async.

I don't want to repeat loads of code everywhere and create two versions of each method, wasting time and effort and reducing maintanability.

I'm guessing I can do something like one of the following:

// Call it just like any other method... 
// the return type would be Task though which isn't right.
DoStuffAsync();

// Wrap it in a Task.Run(...) call, this seems ok, but 
// but is doesn't block, and Task.Run(...) itself returns an
// awaitable Task... back to square one.
Task.Run(() => DoStuffAsync());

// Looks like it might work, but what if I want to return a value?
DoStuffAsync().Wait();

What is the correct way to do this?

UPDATE

Thanks for your answers. The general advice seems to be just provide an Async method and let the consumer decide, rather than creating a wrapper method.

However, let me try to explain my real-world problem...

I have a UnitOfWork class that wraps the the SaveChanges() and SaveChangesAsync() methods of an EF6 DbContext. I also have a List<MailMessage> of emails that need to be sent only when the SaveChanges() succeeds. So my code looks like this:

private readonly IDbContext _ctx;
private readonly IEmailService _email;
private readonly List<MailMessage> _emailQueue;

// ....other stuff    

public void Save() {
    try {
        _ctx.SaveChanges();
        foreach (var email in _emailQueue) _email.SendAsync(email).Wait();

    } catch {
        throw;
    }
}

public async Task SaveAsync() {
    try {
        await _ctx.SaveChangesAsync();
        foreach (var email in _emailQueue) await _email.SendAsync(email);

    } catch {
        throw;
    }
}

So you can see that because an EF6 DbContext provides an async and non-async version, I kinda have to as well... Based on the answers so far, I've added .Wait() to my SendAsync(email) method, which asynchronously sends out an email via SMTP.

So... all good, apart from the deadlock problem... how do I avoid a deadlock?

10
  • 3
    or DoStuffAsync().Result; Commented Apr 22, 2015 at 10:16
  • 5
    If the code is inherently async, just offer the async method and let the consumer decide on where or how to block on it. Don't offer a synchronous wrapper method that just does something the consumer can do for themselves. Commented Apr 22, 2015 at 10:17
  • In fact, Stephen Toub wrote a great blog post on this very subject. Commented Apr 22, 2015 at 10:18
  • @Damien_The_Unbeliever: Good point... but I still need to know how to do it as I'm also writing the comsumer code as well :) Commented Apr 22, 2015 at 10:20
  • I think the "correct" way is to make the consumer async as well, slowly bubbling up until you reach the event listener from your UI. I guess that it doesn't answer your question about how to block in synchronous code though. Commented Apr 22, 2015 at 10:24

3 Answers 3

3

What is the correct way to do this?

The correct way would be to do what you don't want to do, expose two different methods, one which is completely synchronous and another which is completely asynchronous.

Exposing async over sync and sync over async are poth anti-patterns when dealing with async code.

If the methods aren't aren't naturally async, i would expose a synchronous version only and let the invoker decide if he wants to wrap it with a Task.Run himself.

If the method is naturally async, i would expose the async method only. If you have both sync and async versions, expose them independently.

Async methods go "all the way", and they slowly bubble up and down the call stack as you use them. Embrace that fact, and you'll avoid many pitfalls along the way.

Sign up to request clarification or add additional context in comments.

Comments

1

There is a Github project for that : AsyncBridge

Example included in the project homepage :

public async Task<string> AsyncString()
{
    await Task.Delay(1000);
    return "TestAsync";
}

public void Test()
{
    string string1 = "";
    string string2 = "";
    using (var A = AsyncHelper.Wait)
    {
        A.Run(AsyncString(), res => string1 = res);
        A.Run(AsyncString(), res => string2 = res);
    }
}

I used it on several project without issue.

Comments

0

How do I do a blocking wait.

// Looks like it might work, but what if I want to return a value?
DoStuffAsync().Wait();

And it does. Regarding the

what if I want to return a value?

Suppose you also had this async method that returned a Task<string>:

public async Task<string> MyStringGetterAsync() {
    return await GetStringAsync();
}  

You could to the following to perform a blocking wait and retrieve its result:

var myString = MyStringGetterAsync().Result;

With regards to your intention:

I want to provide two methods with the same functionality: an async method which runs asynchronously, and a non-async method that blocks, and either of these two methods can be called depending on whether the code I'm calling from is also async.

Creating a MyWhatEver() overload that just does MyWhatEverAsync.Wait() is not a recommended pattern, because it may have unintended side effects for clients calling that method, as explained e.g. here. The client calling your synchronous version of the method, might not expect its thread to be blocked as the result of calling your method, and cannot decide for itself if that is acceptable (e.g. it cannot specify ConfigureAwait). As mentioned by @Default in the comments, this could lead to deadlocks.

The same is generally true for the reverse case: i.e. having a MyWhatEverAsync() that only does return Task.Run(() => MyWhatEver()); If a client needs to, they could do this themselves and include e.g. a cancellation token, so they can control the task.

It is better to be explicit in your interface and only provide methods that do what their signature says they do.

For your real world problem (after question edit):

I have a UnitOfWork class that wraps the the SaveChanges() and SaveChangesAsync() methods of an EF6 DbContext. I also have a List<MailMessage> of emails that need to be sent only when the SaveChanges() succeeds.

And (from the comments)

The success of sending an email isn't critical.

Your current UnitOfWork design for the Save and SaveAsync methods is not correct, because it violates the atomicity promises it makes.

In the scenario where "SaveChanges" succeeds, but an exception occurs on sending one of the emails. The client code calling one of the Save variants, will observe that exception, will (and should) assume that saving the changes failed, and could retry applying them.

If a client was wrapping this operation in a transaction context with a two phase commit, the "SaveChanges" would be rolled back, and when a client retries the operation, it will attempt to send the emails again, potentially causing a number of recipients to receive the email multiple times, although the changes they received the email about were not committed successfully (and may never be if sending emails fails repeatedly).

So you will need to change something.

If you really don't care about the success of sending the emails, I suggest you add the following method.

private Task SendEmailsAndIgnoreErrors(IEmailService emailService, List<MailMessage> emailQueue) {
    return Task.WhenAll(emailQueue.Select(async (m) => {
        try {
            await emailService.SendAsync(m).ConfigureAwait(false);
        } catch {
            // You may want to log the failure though,
            // or do something if they all/continuously fail.
        }
    }));
}

And use it like this:

public async Task SaveAsync() {
    await _ctx.SaveChangesAsync();
    await SendEmailsAndIgnoreErrors(_email, _emailQueue);
}

I would personally omit the synchronous variant, and let clients decide how to wait on the task, but if you believe you really need it, you could do this:

public void Save() {
    _ctx.SaveChanges();
    SendEmailsAndIgnoreErrors(_email, _emailQueue).Wait().ConfigureAwait(false);
}

3 Comments

@Default agreed, was still busy adding the second part of the answer, related to why you might not want to do this.
@Alex: Thanks for your answer... I've updated my question with an explaintion of why I'm trying to do this... how do I avoid a deadlock?

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.