1

I'm using a client library for accessing a 3rd party API. The library was generated by NSwagStudio from Swagger documentation.

The app I'm working on is entirely synchronous in all its calls and updating it to be async is out of scope of what I'm working on.

When I test the client library from a unit test, it works fine. When I try to call it from within an ASP.Net app, I get the following error:

The CancellationTokenSource has been disposed.

I've distilled the client library down to the essentials for demonstrating the problem, I selected an option to provide sync methods as well as async:

public class ClientApi
{
    private readonly HttpClient _httpClient;

    public ClientApi(HttpClient httpClient)
    {
        _httpClient = httpClient;
    }

    public string BaseUrl { get; set; }

    public object Get()
    {
        return Task.Run(async () => await GetAsync(CancellationToken.None)).GetAwaiter().GetResult();
    }

    /// <returns>OK</returns>
    /// <param name="cancellationToken">
    ///     A cancellation token that can be used by other objects or threads to receive notice of
    ///     cancellation.
    /// </param>
    public async Task<string> GetAsync(CancellationToken cancellationToken)
    {
        var client_ = _httpClient;
        try
        {
            using (var request_ = new HttpRequestMessage())
            {
                request_.Method = new HttpMethod("GET");
                request_.RequestUri = new System.Uri(BaseUrl, System.UriKind.RelativeOrAbsolute);

                var response_ = await client_.SendAsync(
                    request_, 
                    HttpCompletionOption.ResponseHeadersRead, 
                    cancellationToken
                ).ConfigureAwait(false);

                try
                {
                    // Exception occurs on following line
                    var responseData_ = response_.Content == null 
                        ? null 
                        : await response_.Content.ReadAsStringAsync().ConfigureAwait(false);
                    return responseData_;
                }
                finally
                {
                    response_?.Dispose();
                }
            }
        }
        finally { }
    }
}

Here's the code that calls it:

    protected void OnClick(object sender, EventArgs e)
    {
        var httpClient = new HttpClient();

        var client = new ClientApi(httpClient)
        {
            BaseUrl = "https://www.google.com"
        };

        var html = client.Get();
    }

The code calling this is just an asp.net page with a button, and the button events runs the same code as the unit test that passes.

When I compare the runs in a debugger: from a unit test, the response_.Content object does not have a cancellation token, however when run from asp.net it does. In fact they almost seem to be different objects, despite the fact GetType() reports them both as being System.Net.Http.StreamContent. From decompiling the class, this doesn't have a _cancellationtoken property, so where is the debugger getting it from?

From Asp.Net

From Unit Test

I'm guessing that the http request to my asp.net web app has it's own token and source, that is somehow getting used by the HttpClient. However, the client is awaiting all the async calls to get the result synchronously, so I don't understand how the underlying CTS could be disposed as we haven't returned from the call the client library yet.

Can anyone understand what's happening and is there a resolution?

TheLogicMan
  • 371
  • 4
  • 12
  • 2
    you probably don't want to use await + GetResult/.Result together. This combination could easily deadlock – Steve Sep 11 '18 at 15:23
  • It's not my code as it's generated by NSwag Studio. What is the recommended way to call an async method synchronously to avoid potential deadlocks @steve? – TheLogicMan Sep 11 '18 at 15:50
  • you don't. either synch all the way or async all the way. mixing them can easily result in deadlock – Steve Sep 11 '18 at 17:07
  • Changing your event handler to be async should be pretty easy. It's an event, and it's okay for framework events to be async void (it's actually one of the very few cases when that's ok). – mason Sep 11 '18 at 18:07

1 Answers1

0

First of, you should really rethink of rewriting your client app so you can implement async all the way.

“Async all the way” means that you shouldn’t mix synchronous and asynchronous code without carefully considering the consequences. In particular, it’s usually a bad idea to block on async code by calling Task.Wait or Task.Result.

Taken from this great guide.

Basicaly, by running async code sync you will allways do things wrong.

But if you really need one solution, start by wrapping your disposable objects in using statements instead of manually disposing them. Here's a simplified solutions of your ClientApi class which does what you need(But it can deadlock). The code is basically the same as in this answer.

public class ClientApi
{
    public object Get(string url)
    {
        using (var client = new HttpClient())
        {
            var response = client.GetAsync(url).Result;
            if (response.IsSuccessStatusCode)
            {
                var responseContent = response.Content;
                return responseContent.ReadAsStringAsync().Result;
            }
        }

    }
}

Read more about deadlock here

Marcus Höglund
  • 16,172
  • 11
  • 47
  • 69
  • 1
    Everything I've read about HttpClient suggests it should live the lifetime of your application and creating a new instance per request is a bad idea that leads to socket problems when your app starts to make a load of requests. I specifically chose an option when generating the client that allowed me to inject in a client instead so I could so this. – TheLogicMan Sep 12 '18 at 11:47
  • I've ditched the generated client and am now using a simple client I wrote using RestSharp and sync only calls. However, I raised this issue as I'm hoping to learn about why I'm seeing this behaviour in this specific example. How can an object (StreamContent) sometimes have a property and not other times. This baffles me and goes against how I understand the language to work so would love some enlightenment! – TheLogicMan Sep 12 '18 at 11:54