1

One thing that I understood the best practice in using HttpClient is to use a single instance of it in my project (e.g, Here)

Now I have an ASP.NET MVC project where I am using HttpClient to consume a REST API. I created a client like below:

public class MyClient {
   HttpClient client = new HttpClient();

   public MyClient() {
      client.BaseAddress = new Uri("http://localhost.fiddler/1/");
      //
   }

   public async Task<SubscriptionResponse> Subscribe(SubscriptionRequest request, string serviceId) {
      client.DefaultRequestHeaders.Remove("X-RequestHeader");
      client.DefaultRequestHeaders.Add("X-RequestHeader", $"request ServiceId=\"{serviceId}\"");

      SubscriptionResponse subscriptionResp = null;
      HttpResponseMessage response = await client
            .PostAsJsonAsync("subscriptions/index.php", subscription);

      if (response.IsSuccessStatusCode)
      {
          subscriptionResp = await response.Content.ReadAsAsync<SubscriptionResponse>();
      }
      return subscriptionResp;
   }
}

So in order to use a single instance of HttpClient I am going to use a dependency container to inject a singleton instance of MyClient in my controllers. And I will use it in actions like below:

public MyClient Client {get; set;} // Will be injected by a dependency container

public async Task<ActionResult> CallApi(SubscriptionRequest req, string serviceId) {
   await Client.Subscribe(request, serviceId);
   return View();
}

Now my question is that am I using HttpClient safely? I don't have any experience regarding code thread safety and my emotions are telling me that there is something wrong here :D

I am specifically talking about this section in Subscribe method:

client.DefaultRequestHeaders.Remove("X-RequestHeader");
client.DefaultRequestHeaders.Add("X-RequestHeader", $"request ServiceId=\"{serviceId}\"");

UPDATE I am concerned about this as multiple threads may call this method at the same time.

Mustafa Shujaie
  • 1,447
  • 1
  • 16
  • 30
  • Does `MyClient` implement `IDisposable`? – Jasen Jul 12 '17 at 17:43
  • @Jasen Yes, but my concern is not about that. Please see my updated question. – Mustafa Shujaie Jul 12 '17 at 17:47
  • You should make your HttpClient static – maccettura Jul 12 '17 at 17:51
  • 1
    Well yep, the last thread to set the header value before the post wins, See [HttpClient single instance with different authentication headers](https://stackoverflow.com/questions/37928543/httpclient-single-instance-with-different-authentication-headers) – Alex K. Jul 12 '17 at 17:51
  • @AlexK. Yes that is what my emotions are telling me :) – Mustafa Shujaie Jul 12 '17 at 17:52
  • From https://msdn.microsoft.com/en-us/library/system.net.http.httpclient(v=vs.118).aspx _Thread Safety Any public static (Shared in Visual Basic) members of this type are thread safe. Any instance members are not guaranteed to be thread safe._ – Jasen Jul 12 '17 at 17:52
  • 1
    @Jasen .Net 4.5 changed that & recommends a single shared instance: https://msdn.microsoft.com/en-us/library/system.net.http.httpclient(v=vs.110).aspx - see Remarks – Alex K. Jul 12 '17 at 17:53
  • I understand the shared instance but I don't understand how that makes the thread safety notation invalid. – Jasen Jul 12 '17 at 17:57

0 Answers0