25

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.

Improve this question
0

2 Answers 2

Reset to default
33

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.

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

9 Comments

Is public Task<string> RetrieveHolidayDatesFromSourceAsync() missing an await?
@NickWeaver: No. It's not async, so it can't use await.
I see. I was wondering since the method's name states "Async" at the end. Why is it not set to async?
@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.
Had same expectation as @NickWeaver ... so returning a Task<Generic> 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 <del>await</del> get the value out of the Task if it's *not awaitable? Checking here
|
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.

Improve this answer

7 Comments

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)".
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?
"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.
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.
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.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.