HttpClient Singleton Implementation in ASP.NET MVC

Question

After reading this blog post and thisofficial note on www.asp.net:

HttpClient is intended to be instantiated once and re-used throughout the life of an application. Especially in server applications, creating a new HttpClient instance for every request will exhaust the number of sockets available under heavy loads. This will result in SocketException errors.

I discovered that our code was disposing the HttpClient on each call. I'm updating our code so that we reuse the HttClient, but I'm concerned our implement but not thread-safe.

Here is the current draft of new code:

For Unit Testing, we implemented an wrapper for HttpClient, the consumers call the wrapper:

 public class HttpClientWrapper : IHttpClient
    {
        private readonly HttpClient _client;

        public Uri BaseAddress
        {
            get
            {
                return _client.BaseAddress;
            }

            set
            {
                _client.BaseAddress = value;
            }
        }

        public HttpRequestHeaders DefaultRequestHeaders
        {
            get
            {
                return _client.DefaultRequestHeaders;
            }
        }

        public HttpClientWrapper()
        {
            _client = new HttpClient();
        }

        public Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, String userOrProcessName)
        {
            IUnityContainer container = UnityCommon.GetContainer();
            ILogService logService = container.Resolve<ILogService>();

            logService.Log(ApplicationLogTypes.Debug, JsonConvert.SerializeObject(request), userOrProcessName);

            return _client.SendAsync(request);
        }

        #region IDisposable Support
        private bool disposedValue = false; // To detect redundant calls

        protected virtual void Dispose(bool disposing)
        {
            if (!disposedValue)
            {
                if (disposing && _client != null)
                {
                    _client.Dispose();
                }

                disposedValue = true;
            }
        }

        public void Dispose()
        {
            Dispose(true);
        }
        #endregion
    } 

Here is a service that calls:

public class EnterpriseApiService : IEnterpriseApiService
    {
        private static IHttpClient _client;

        static EnterpriseApiService()
        {
            IUnityContainer container = UnityCommon.GetContainer();
            IApplicationSettingService appSettingService = container.Resolve<IApplicationSettingService>();

            _client = container.Resolve<IHttpClient>();
        }

        public EnterpriseApiService() { }

        public Task<HttpResponseMessage> CallApiAsync(Uri uri, HttpMethod method, HttpContent content, HttpRequestHeaders requestHeaders, bool addJsonMimeAccept = true)
        {
            IUnityContainer container = UnityCommon.GetContainer();
            HttpRequestMessage request;

           _client.BaseAddress = new Uri(uri.GetLeftPart(UriPartial.Authority));

            if (addJsonMimeAccept)
                _client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));

            request = new HttpRequestMessage(method, uri.AbsoluteUri);

            // Removed logic that built request with content, requestHeaders and method

            return _client.SendAsync(request, UserOrProcessName);
        }
    }

My questions:

1. Is this an appropriate approach to reuse the HttpClient object?
2. Is the static _httpClient field (populated with the static constructor) shared for all instances of EnterpriseApiService? I wanted to confirm since is being called by instance methods.
3. When CallApiAsync() is called, when that makes changes to the static HttpClient, such as the "_client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"))" could those values be overwriten by another process before the last line "_client.SendAsync" is called? I'm concerned that halfway through processing CallApiAsync() the static instance is updated.
4. Since it is calling SendAsync(), are we guaranteed the response is mapped back to the correct caller? I want to confirm the response doesn't go to another caller.

Update: Since I've removed the USING statements, and the Garage Collection doesn't call Dispose, I'm going to go with the safer approach of creating a new instance within the method. To reuse an instance of HttpClient even within the thread lifetime, it would require a significant reworking of the logic because the method sets HttpClient properties per call.

Answer 1

Do you really want one instance?

I don't think you want one instance application-wide. You want one instance per thread. Otherwise you won't get very good performance! Also, this will resolve your questions #3 and #4, since no two threads will be accessing the same HttpClient at the same time.

You don't need a singleton

Just use Container.Resolve with the PerThreadLifetimeManager.

Answer 2

For those lucky enough to be using .NET Core this is fairly straightforward.

As John Wu so eloquently stated, you don't want a singleton per se, but rather a singleton per request. As such, the AddScoped<TService>() method is what you're after.

In your ConfigureServices(IServiceCollection services) method:

services.AddScoped<HttpClient>();

To consume:

public class HomeController 
{
    readonly HttpClient client;

    public HomeController (HttpClient client)   
    {
        this.client = client;
    }

    //rest of controller code
}

Answer 3

Since it is calling SendAsync(), are we guaranteed the response is mapped back to the correct caller? I want to confirm the response doesn't go to another caller.

This will be handled via callback pointers. It has nothing to do with using HttpClient as singleton. More details here - https://stackoverflow.com/a/42307650/895724

Answer 4

This is what i use

public abstract class BaseClient : IDisposable
{

    private static object locker = new object();
    private static volatile HttpClient httpClient;

    protected static HttpClient Client
    {
        get
        {
            if (httpClient == null)
            {
                lock (locker)
                {
                    if (httpClient == null)
                    {
                        httpClient = new HttpClient();
                    }
                }
            }

            return httpClient;
        }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            if (httpClient != null)
            {
                httpClient.Dispose();
            }

            httpClient = null;
        }
    }
}

Its used in the extension method like this:

public static Task<HttpResponseMessage> PostAsJsonAsync<T>(
        this HttpClient httpClient, string url, T data, string token, IDictionary<string, string> dsCustomHeaders = null)
    {
        ThrowExceptionIf.Argument.IsNull(httpClient, nameof(httpClient));
        var dataAsString = JsonConvert.SerializeObject(data);            
        var httpReqPostMsg = new HttpRequestMessage(HttpMethod.Post, url)
        {
            Content = new StringContent(dataAsString, Encoding.UTF8, "application/json")
        };
        httpReqPostMsg.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token);
        httpReqPostMsg.Headers.Add(Constants.TelemetryCorrelationKey, Utilities.GetRequestCorrelationId());
        if (dsCustomHeaders != null) { 
            foreach (var keyValue in dsCustomHeaders)
            {
                httpReqPostMsg.Headers.Add(keyValue.Key, keyValue.Value);
            }
        }
        return httpClient.SendAsync(httpReqPostMsg);
    }