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.
I tried to reuse entity framework context as if it was db connection and it ended in eveything working but occasionally producing bizzare errors that went away only when I created at least one context per method.
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.
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."
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).
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.
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.
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);
}
14 comments
[ 2.7 ms ] story [ 43.4 ms ] threadWe changed the title in an attempt to make it more accurate and neutral.
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.
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.
"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).
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.
In a typical usage scenario in an async Task, this is what it'd look like: