24

I have the below method:

    public string RetrieveHolidayDatesFromSource() {
        var result = this.RetrieveHolidayDatesFromSourceAsync();
        /** Do stuff **/
        var returnedResult  = this.TransformResults(result.Result); /** Where result gets used **/
        return returnedResult;
    }


    private async Task<string> RetrieveHolidayDatesFromSourceAsync() {
        using (var httpClient = new HttpClient()) {
            var json = await httpClient.GetStringAsync(SourceURI);
            return json;
        }
    }

The above does not work and seems to not return any results properly. I am not sure where I am missing a statement to force the await of a result? I want the RetrieveHolidayDatesFromSource() method to return a string.

The below works fine but it is synchronous and I believe it can be improved upon? Note that the below is synchronous in which I would like to change to Asynchronous but am unable to wrap my head around for some reason.

    public string RetrieveHolidayDatesFromSource() {
        var result = this.RetrieveHolidayDatesFromSourceAsync();
        /** Do Stuff **/

        var returnedResult = this.TransformResults(result); /** This is where Result is actually used**/
        return returnedResult;
    }


    private string RetrieveHolidayDatesFromSourceAsync() {
        using (var httpClient = new HttpClient()) {
            var json = httpClient.GetStringAsync(SourceURI);
            return json.Result;
        }
    }

Am I missing something?

Note: For some reason, when I breakpoint the above Async Method, when it gets to the line var json = await httpClient.GetStringAsync(SourceURI) it just goes out of breakpoint and I can't go back into the method.

Selim Yildiz
  • 5,254
  • 6
  • 18
  • 28
Samuel Tambunan
  • 335
  • 1
  • 2
  • 11

2 Answers2

31

Am I missing something?

Yes. Asynchronous code - by its nature - implies that the current thread is not used while the operation is in progress. Synchronous code - by its nature - implies that the current thread is blocked while the operation is in progress. This is why calling asynchronous code from synchronous code literally doesn't even make sense. In fact, as I describe on my blog, a naive approach (using Result/Wait) can easily result in deadlocks.

The first thing to consider is: should my API be synchronous or asynchronous? If it deals with I/O (as in this example), it should be asynchronous. So, this would be a more appropriate design:

public async Task<string> RetrieveHolidayDatesFromSourceAsync() {
    var result = await this.DoRetrieveHolidayDatesFromSourceAsync();
    /** Do stuff **/
    var returnedResult  = this.TransformResults(result); /** Where result gets used **/
    return returnedResult;
}

As I describe in my async best practices article, you should go "async all the way". If you don't, you won't get any benefit out of async anyway, so why bother?

But let's say that you're interested in eventually going async, but right now you can't change everything, you just want to change part of your app. That's a pretty common situation.

In that case, the proper approach is to expose both synchronous and asynchronous APIs. Eventually, after all the other code is upgraded, the synchronous APIs can be removed. I explore a variety of options for this kind of scenario in my article on brownfield async development; my personal favorite is the "bool parameter hack", which would look like this:

public string RetrieveHolidayDatesFromSource() {
  return this.DoRetrieveHolidayDatesFromSourceAsync(sync: true).GetAwaiter().GetResult();
}

public Task<string> RetrieveHolidayDatesFromSourceAsync() {
  return this.DoRetrieveHolidayDatesFromSourceAsync(sync: false);
}

private async Task<string> DoRetrieveHolidayDatesFromSourceAsync(bool sync) {
  var result = await this.GetHolidayDatesAsync(sync);
  /** Do stuff **/
  var returnedResult  = this.TransformResults(result);
  return returnedResult;
}

private async Task<string> GetHolidayDatesAsync(bool sync) {
  using (var client = new WebClient()) {
    return sync
        ? client.DownloadString(SourceURI)
        : await client.DownloadStringTaskAsync(SourceURI);
  }
}

This approach avoids code duplication and also avoids any deadlock or reentrancy problems common with other "sync-over-async" antipattern solutions.

Note that I would still treat the resulting code as an "intermediate step" on the path to a properly-asynchronous API. In particular, the inner code had to fall back on WebClient (which supports both sync and async) instead of the preferred HttpClient (which only supports async). Once all the calling code is changed to use RetrieveHolidayDatesFromSourceAsync and not RetrieveHolidayDatesFromSource, then I'd revisit this and remove all the tech debt, changing it to use HttpClient and be async-only.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 1
    Is `public Task RetrieveHolidayDatesFromSourceAsync()` missing an `await`? – Nick Weaver Dec 07 '16 at 09:31
  • 3
    @NickWeaver: No. It's not `async`, so it can't use `await`. – Stephen Cleary Dec 07 '16 at 14:25
  • I see. I was wondering since the method's name states "Async" at the end. Why is it not set to async? – Nick Weaver Dec 07 '16 at 21:07
  • 1
    @NickWeaver: Since it's a trivial method, it just returns the task directly. It is still an asynchronous API, hence the `Async` suffix, but does not use `async`/`await` for its implementation. – Stephen Cleary Dec 07 '16 at 22:17
  • 1
    Thanks for the explanation. – Nick Weaver Dec 08 '16 at 08:58
  • Had same expectation as @NickWeaver ... so returning a `Task` means your code can still be asynchronous, or have "an asynchronous API" -- but it need not use `async` / `await`. It **can (and should) ** use those -- and in order to use `await`, the function *must* be marked `async`-- but it **need not** use `async` / `await`. Do I understand correctly? Or if I'm wrong, what's the distinction? And in any case, how to await get the value out of the Task if it's *not* awaitable? [Checking here](https://stackoverflow.com/questions/20884604/get-result-from-async-method) – Nate Anderson May 26 '17 at 18:10
  • @TheRedPea: Yes, there's a difference between "asynchronous" (does not block the calling thread) and `async` (an implementation technique). E.g., a TAP method defined in an interface is just Task-returning - no `async`. The implementation may or may not use `async`. Also, `async` does not "make a task awaitable". `Task` is an awaitable type, period, whether it's created by the `async` state machine or by something else. – Stephen Cleary May 26 '17 at 23:25
  • OK, I got confused , **my** method called [`await GetChildContentAsync()`](https://github.com/aspnet/Announcements/issues/90) because of compiler warning telling me to "...Consider marking this method with the 'async' modifier..." . Because "this" is ambiguous, I assumed an issue with `GetChildContentAsync` (which is red-squiggly-underlined) method, and when I did F12 to check implementation, saw the metadata view, which **did** show me a `Task` is returned, but **did not** show me the `async` keyword. In fact I was missing the `async` on **my** method (duh). – Nate Anderson May 26 '17 at 23:36
  • When I try this pattern I get the error `There is no implicit conversion between 'Task' and 'string'`. Does this code no longer work in the newest version of C#? – inejwstine Sep 30 '19 at 22:36
  • Nevermind. I was calling the same method in both sides of the conditional operator, rather than the sync version in one and async in the other. – inejwstine Sep 30 '19 at 23:47
7
public string RetrieveHolidayDatesFromSource() {
    var result = this.RetrieveHolidayDatesFromSourceAsync().Result;
    /** Do stuff **/
    var returnedResult  = this.TransformResults(result.Result); /** Where result gets used **/
    return returnedResult;
}

If you add .Result to the async call, it will execute and wait for the result to arrive, forcing it to be synchronous

UPDATE:

private static string stringTest()
{
    return getStringAsync().Result;
}

private static async Task<string> getStringAsync()
{
    return await Task.FromResult<string>("Hello");
}
static void Main(string[] args)
{
    Console.WriteLine(stringTest());

}

To address the comment: This works without any problems.

Pedro G. Dias
  • 3,162
  • 1
  • 18
  • 30
  • 1
    I don't think this is working as I already have the "result.Result" in the method. Having this.RetrieveHolidayDatesFromSourceAsync().Result will change result into a non-task object which will cause an error in "var returnedResult = this.TransformResults(result.Result)". – Samuel Tambunan Apr 01 '16 at 05:45
  • I updated my answer to show you working code. You can see getStringAsync is an async method that is called in a synced method (stringTest()), I Get the desired output without errors. What is the error you're getting? – Pedro G. Dias Apr 01 '16 at 06:02
  • 6
    "This works without any problems" :)... you may be a bit nicer and explain how to deal with resulting deadlock when actually used outside console apps. – Alexei Levenkov Apr 01 '16 at 06:26
  • If there is a deadlock there, I can't see it. – Pedro G. Dias Apr 01 '16 at 06:27
  • 2
    There may be one in case you run in a UI environment or any environment with a single thread `SynchronizationContext`. In such a case the call to `.Result` will block the further execution but the `await` in the `async` method may register it's continuation later during the runtime of the thread that is blocked. It is always pretty dangerous to mix asynchronous and synchronous waiting, especially in UI environments. – Nitram Apr 01 '16 at 08:58
  • 1
    The fact that it does "work without any problems" is a bit of a fluke and is solely due to the fact that the returned task is already completed when you await it. This would not be the case with any code which uses `HttpClient` or `WebClient`. – Kirill Shlenskiy Apr 04 '16 at 00:14
  • I want to call an async API that I do not control (It is async, and I cannot change it.) from an existing .aspx page with lots of user controls. I can make the .aspx page async, but do I then have to make all the user controls async, and also all the other pages that use the user controls? – Jim S Nov 16 '17 at 02:19