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?

BG100
  • 4,481
  • 2
  • 37
  • 64
  • 3
    or `DoStuffAsync().Result;` – Milen Apr 22 '15 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. – Damien_The_Unbeliever Apr 22 '15 at 10:17
  • In fact, Stephen Toub wrote a great [blog post](http://blogs.msdn.com/b/pfxteam/archive/2012/04/13/10293638.aspx) on this very subject. – Damien_The_Unbeliever Apr 22 '15 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 :) – BG100 Apr 22 '15 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. – default Apr 22 '15 at 10:24
  • I'd suggest Yuval has provided the correct answer below. For the sake of completeness Stephen Clearly provides an additional alternative to the options you listed in your question - http://blog.stephencleary.com/2014/12/a-tour-of-task-part-6-results.html. – Daniel Kelley Apr 22 '15 at 10:41
  • @BG100 When `SaveChanges` succeeds and sending one of the emails fails, what do you do? Rollback the unit of work? How do you rollback sending an email? It seems that you are "conflating" two separate concerns in the unit of work. It would be better to either (1) move the sending of the emails out of the method, (2) return the email sending part as a separate task after the try catch block. – Alex Apr 22 '15 at 11:23
  • @Alex: The success of sending an email isn't critical... but I guess I could wrap it in a transaction. So I'm happy with doing it like this. – BG100 Apr 22 '15 at 11:35
  • @BG100 Ok, so you are not rolling back the "Save Changes" as part of a transaction context when an exception occurs on sending one of the emails. Your current "unit of work" design breaks its atomic (everything succeeds or fails) promise to its clients. If the "Save Changes" succeeds, but sending an email fails with an exception (that the client will see as the result of the unit of work), the client should think that the unit of work failed, and retry, when in fact part of it (the part you care about most so it seems) succeeded, and should not be retried. – Alex Apr 22 '15 at 12:09
  • @Alex: ok... thats fair enough, I understand that there are other issues, maybe I need to rename UnitOfWork to something else... but this is how I want it to work. Afterall, if there are two emails in a queue, the first one succeeds and the second fails, how can I possibly rollback the sending of an email? but all this is kinda going beyond the scope of my question... I simply want to know how to safely call an async method from non-async code... even fire and forget would work here. – BG100 Apr 22 '15 at 12:35

3 Answers3

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.

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
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.

Eric Boumendil
  • 2,318
  • 1
  • 27
  • 32
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);
}
Alex
  • 13,024
  • 33
  • 62
  • although, [that has the possibility of creating deadlocks if you are not careful](http://blog.stephencleary.com/2012/07/dont-block-on-async-code.html) – default Apr 22 '15 at 10:22
  • @Default agreed, was still busy adding the second part of the answer, related to why you might not want to do this. – Alex Apr 22 '15 at 10:26
  • @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? – BG100 Apr 22 '15 at 11:17