
Using the .NET HttpClient class wrongly can destabilize software - douche
http://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/
======
dogma1138
To be fair this isn't using the HttpClient incorrectly this is using "OOP" at
least .NET incorrectly.

"Do not initialize unnecessarily" is a pretty common paradigm in .NET this
extends way beyond variables and constructors. Even if the HttpClient did not
cause this http.sys/winsock errata there is still absolutely no reason to
initialize a new instance of it within a loop you are wasting CPU time and
more importantly memory and leaving your garbage collector extra work.

Idealy a loop should not really initialize or create any new objects, create
what you need an access it from within the loop, initializing a new instance
of a class in every cycle of the loop, creating new variables, subscribing to
event handlers etc. is just really really bad coding practice and this is
basic .NET performance practices.

The "lesson" the author should've learned isn't that HttpClient is designed
oddly, or that there are some oddities with how Windows handles sockets, but
that "var foo = new bar()" and the likes do not belong inside a loop or any
other iterative statement unless there is absolutly no other way of doing
without it (and I can't think of a scenario in which that would be the case).

~~~
artpepper
I think the case that trips people up is using HttpClient in an ASP.NET MVC
controller action. There, it doesn't _look_ like a loop, but if you
create/dispose on each request, you can get socket exhaustion under load.

~~~
dogma1138
If that's the case you should use HTTP pipelining usually to bundle multiple
requests via a single tcp connection.

[https://en.m.wikipedia.org/wiki/HTTP_pipelining](https://en.m.wikipedia.org/wiki/HTTP_pipelining)

Have it sitting in the background waiting for the controller to ask it to do
something, you can manage the sockets and you can make a request queue on your
end if you getting into resource cap issues.

------
douche
I found this interesting, because I had just implemented a REST client that
used a single HttpClient for its entire lifecycle, mostly for convenience.
It's good to know I probably shouldn't go back through and change that design.

IDisposable kind of sucks. Unless you know the inner workings of whatever
implements it, it's hard to know how to handle lifecycles and how important
properly disposing is. Is it wrapping a heavyweight COM object that is going
to leak memory badly? Is it going to tie up limited system resources? Or is it
a lightweight .NET object that someone has abused the interface to perform
some trivial cleanup operations? There's no real guarantees.

~~~
pixelbath
With regard to your comment about IDisposable, I just fixed a bug caused by
some wrapped pointer code de-allocating in a Dispose() method. While I thought
I broke the code by simply changing the framework, it turns out this error was
occurring _all the time_ , but only cropped up because .Net 4 treats Corrupted
State Exceptions (CSEs) differently and doesn't catch them by default. I was
shocked that the answer was "the library you've been using for 6 years is
crap."

------
rianjs
HttpResponseMessages also implement IDisposable, and should be disposed.
HttpContent also implements IDisposable, but disposing of the parent
HttpResponseMessage also disposes of the child HttpContent object as step 1,
so you needn't have a second using statement.

In a typical usage scenario in an async Task, this is what it'd look like:

    
    
      using (var response = await _httpClient.GetAsync(requestUri))
      {
          response.EnsureSuccessStatusCode();
          var result = await response.Content.ReadAsStringAsync();
          return JsonConvert.DeserializeObject<Foo>(result);
      }

------
Analemma_
I once attended a course on the .NET memory management and GC model taught by
Jeffrey Richter, who was on the CLR team for some time. His opinion was that
you should _never_ use "using", and that he opposed adding it to the language
at all, but was overridden. At the end of the day I think that's a little
excessive, but I do try to avoid "using" unless I know the surrounding object
has a long lifecycle and the IDisposable utilizes a very limited resource.

~~~
nextweek2
You'll need to explain the reasoning behind why he doesn't want people to use
using.

I've not found a drawback and found it shortened my code. Only downside is the
extra indentation.

The original article just pointed out a basic coding flaw of having new within
a loop. The using wasn't the bad part.

------
generalpf
Nice post but I hate titles like this where the author assumes nobody is doing
it correctly but them.

~~~
mindcrash
All code i saw until now creates new instances of HttpClient everywhere it is
being used. So yeah...

