Why does HttpClient appear to hang without .Result?

Question

I have this code to call an API which returns a token. However, it will only return if I replace this line:

var response = await TokenClient.PostAsync(EnvironmentHelper.TokenUrl, formContent);

with this line:

var response = TokenClient.PostAsync(EnvironmentHelper.TokenUrl, formContent).Result;

Why?

public static async Task<Token> GetAPIToken()
{
    if (DateTime.Now < Token.Expiry)
        return Token;

    TokenClient.DefaultRequestHeaders.Clear();
    TokenClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
    TokenClient.BaseAddress = new Uri(EnvironmentHelper.BaseUrl);
    TokenClient.Timeout = TimeSpan.FromSeconds(3);

    var formContent = new FormUrlEncodedContent(new[]
    {
        new KeyValuePair<string, string>("grant_type", "client_credentials"),
        new KeyValuePair<string, string>("client_id", EnvironmentHelper.APITokenClientId),
        new KeyValuePair<string, string>("client_secret", EnvironmentHelper.APITokenClientSecret)
    });

    try
    {
        // HANGS - this next line will only ever return if suffixed with '.Result'
        var response = await TokenClient.PostAsync(EnvironmentHelper.TokenUrl, formContent);

        var content = await response.Content.ReadAsStringAsync();
        dynamic jsonContent = JsonConvert.DeserializeObject(content);

        Token token = new Token();
        token.AccessToken = jsonContent.access_token;
        token.Type = jsonContent.token_type;
        token.Expiry = DateTimeOffset.Now.AddSeconds(Convert.ToDouble(jsonContent.expires_in.ToString()) - 30);
        token.Scope = Convert.ToString(jsonContent.scope).Split(' ');

        Token = token;
    }
    catch (Exception ex)
    {
        var m = ex.Message;
    }

    return Token;
}

How are you calling `GetAPIToken`? Are you `await`ing all the way up the chain? — DavidG, May 31 '19 at 12:34
The PostAsync does not take a long time to return. This is demonstrated by adding .Result and removing the await. It also responds very quickly in Postman. — Matt W, May 31 '19 at 12:34
@DavidG - Almost. The top-most code where awaiting is not performed has this: var wm = new WebRepo().GetWeb(Id).Result; — Matt W, May 31 '19 at 12:37
When investigating threading issues, you need to look at the entire call chain. Add how is `GetAPIToken` called, who calls that caller, etc, until the end of your code — Camilo Terevinto, May 31 '19 at 12:37
Yup, don't do that. Use `await` all the way or you are in danger of deadlocks like you are seeing. — DavidG, May 31 '19 at 12:37
@MattW HttpClient doesn't need `.Result`, in fact using can *cause* hangs — Panagiotis Kanavos, May 31 '19 at 12:37
@MattW Well, using `await` in a method synchronously blocked with `.Result` tends to cause deadlocks — Camilo Terevinto, May 31 '19 at 12:39
@MattW if your code hangs it means that execution can't resume *after* await because the original execution context is busy. In a desktop application this means that the UI thread is busy doing something else. — Panagiotis Kanavos, May 31 '19 at 12:39
@DavidG Is there a way to wrap one of the calls? It is a mammoth task to go and refactor that much code. — Matt W, May 31 '19 at 12:39
Well, you could look at using `.ConfigureAwait(false)` but that's still only a patch on bad coding practices. — rory.ap, May 31 '19 at 12:41
@MattW the real solution is to remove all `.Result` or `.Wait()` calls, use `async/await` all the way to the top. Another option is to use `ConfigureAwait(false)` which means your code *won't* get back to the UI thread. This means that you won't be able to modify the UI either — Panagiotis Kanavos, May 31 '19 at 12:41
@MattW But if the code "works" as-is, why do you care? Either leave it as-is or modify everything as needed — Camilo Terevinto, May 31 '19 at 12:44
There's a lot of code to modify. Much of it will be very difficult to refactor. It is being changed to add functionality, not because it doesn't work. The root of where the problem code is being called from is in a static property getter. — Matt W, May 31 '19 at 12:52
Your program is most certainly getting into a deadlock depending on the usage of the function. If the calling code is not async then wrap it in a task and await the method call and remove .Result — Jinish, May 31 '19 at 12:55
@Jinish Wrapping in a task isn't always the solution either. — DavidG, May 31 '19 at 12:56
@DavidG Agreed and I do understand. Although without knowing much about the calling code, its kind of difficult to provide with an accurate solution — Jinish, May 31 '19 at 12:57
The calling code is a public static property with a getter, referenced in over 100 places. I'm not sure how best to convert the property to async friendly and, like I said, refactoring the entire application is a huge task. — Matt W, May 31 '19 at 12:59

score 0 · Accepted Answer · answered May 31 '19 at 13:38

The top-most code where awaiting is not performed has this: var wm = new WebRepo().GetWeb(Id).Result

You're running into a deadlock since the synchronous code is blocking on async code. The best solution is to remove the blocking and go async all the way.

The calling code is a public static property with a getter, referenced in over 100 places. I'm not sure how best to convert the property to async friendly

There are a few approaches to async properties, depending on what the property's semantics are. If it re-evaluates each time it's called, then it's really a method in disguise and should become an async Task<T> method. If it is set once, then AsyncLazy<T> may be a better choice.

Is there a way to wrap one of the calls? It is a mammoth task to go and refactor that much code.

There's no wrapping that works for all scenarios. However, there are some techniques for mixing sync and async code that can work for specific scenarios.

Here's what I would consider, in order of precedence:

Make it async all the way. Refactor the code and make it better. (Requires lots of code changes, which may not be feasible at this time).
Make it sync all the way. Change GetAPIToken (and its callers) to be sync instead of async. (Requires all calling code to be made synchronous, which may not be possible).
Use the boolean argument hack to make GetAPIToken callable both synchronously and asynchronously. (Requires the ability to change GetAPIToken).
Use the thread pool hack to run GetAPIToken asynchronously on a thread pool thread and sycnhronously block another thread on it.

Really seems like the real challenge is with the only real guarantee being to make the entire application awaited conflicting with the nature of legacy code. — Matt W, May 31 '19 at 14:12

Why does HttpClient appear to hang without .Result?

1 Answers1