Should WebException.Response.GetResponseStream() be close / disposed?

Asked 1/10, 2015 at 15:40 Answered 31/3, 2017 at 2:3

Solved c#.net idisposable httpwebresponse webexception

When I catch a .NET WebException, should I close / dispose the Response.GetResponseStream()?

The MSDN example does not close or dispose anything in the exception.

Many SO answers recommend disposing the response and / or the stream.

I disposed the stream and this caused big problems. Because GetResponseStream() (always? / sometimes?) returns the same instance. So when I get the response stream and then dispose it, then maybe rethrow the exception to another layer that also gets the response stream it will be already disposed and unreadable and throw more exceptions because of that.

Thin answered 1/10, 2015 at 15:40 Comment(1)

Possible duplicate of How to properly dispose of a WebResponse instance? – Potpie 26/2, 2018 at 8:10

You should dispose of that stream because it might hold resources. But only dispose of it when you are done with it. Simply stop disposing it before you no longer need the stream. Make the last user of the stream dispose of it.

Probably, you should call GetResponseStream() only once and pass the stream around explicitly so that it is clear that it's the same stream.

Flu answered 1/10, 2015 at 16:18 Comment(18)

Can you share a source for this information? Do you see anything in MSDN that suggests this? Do you have extensive experience with GetResponseStream? Or is it just a general guess for how it probably works? Based on my experience guessing doesn't turn out well with this API. – Thin 2/10, 2015 at 7:25

If you don't have information to the contrary all disposable resources must be disposed. Even if their current implementation does not require it (and I'm not saying that is the case here) it might change in the future. Therefore, decompiling code is not a very reliable answer. The GetResponseStream seems very likely to hold resources. If you want to skip disposing you have to be very sure. – Flu 2/10, 2015 at 10:1

Note, that if you guess wrong the result is catastrophic. Random resource exhaustion, slowdown or even hangs in production. Triggered by load, not found during developer testing (because it's not under load). – Flu 2/10, 2015 at 10:1

Sure, but double dispose or dispose before use is also catastrophic. 100% agree that decompiling is unreliable. IMO the API should document by whom and when the resource should be disposed. Especially if it hands out the same resource instance again and again. – Thin 2/10, 2015 at 10:42

Double dispose is always safe in .NET by convention. Disposing an object does not delete it. – Flu 2/10, 2015 at 10:50

But it closes the resource and makes in unreadable. So another layer that also catches the exception and reads the response will fail. – Thin 2/10, 2015 at 10:53

That's true but the problem is not double dispose here. It's early dispose. Here's an architectural idea: Create a class that keeps a ref count for an IDisposable object. That way you can pass around the stream in wild ways and once the last owner is done it get's disposed. So far I never had this need but it might help in your case. – Flu 2/10, 2015 at 11:0

I remember cases where double dispose was an issue, but that may have been a special case of one .NET library (SlimDX). My concern is also that .NET may still be using the response internally without my knowledge. (It's retained internally after all.) – Thin 2/10, 2015 at 11:4

If disposing a publicly exposed object broke the BCL's code that would be a bug in the framework. Very unlikely. Approximately 1000000000000 HTTP requests have been sent using these classes. Bugs are unlikely. – Flu 2/10, 2015 at 11:14

Framework bugs are not that unlikely in my experience. Do you personally know if (a sizeable portion of) those 1000000000000 HTTP requests disposed the response in a webexception? That would indeed be somewhat reassuring. – Thin 2/10, 2015 at 13:11

Framework bugs are common, .NET Framework bugs are rare. This is one of the most used and tested pieces of software in existence. Even before the first release of any piece of code Microsoft applies very high quality standards. The standards are much higher than yours with 95% probability. – Flu 2/10, 2015 at 13:13

I've personally experienced quite a few. Anyway, so you have no direct experience with disposing webexception.response? – Thin 2/10, 2015 at 13:17

I have. If you are worried I suggest you run a test. This discussion is no longer productive. You are preoccupied with the idea that double disposing or badly ordered disposing is somehow dangerous. – Flu 2/10, 2015 at 13:29

Because it is. Exactly that caused a problem in a real world scenario. I'm looking for a way to fix that problem without introducing more. But you are right this is unproductive. – Thin 2/10, 2015 at 13:33

Show me code that demonstrates a bug in the .NET Framework. I doubt there is one. The bug is yours. If you post code I can identify it. – Flu 2/10, 2015 at 13:34

Sorry, you misunderood me. I'm not saying there is a .NET Framework bug. I'm saying the documentation is silent on the matter of disposing the response (&stream). The code in question did dispose it. This caused problems: after rethrowing the exception, recatching it elsewher, re-calling Response.GetResponseStream() there again, the same disposed stream is returned again and reading from it fails. All this is already described in the original question. – Thin 2/10, 2015 at 13:41

Specifically: You say "Make the last user of the stream dispose of it." OK, how do I know I'm the last user of the stream? The documentation does not say if GetResponseStream() always returns the same stream or not. If I call it twice should I dispose it twice? Is it OK to call it zero times and not dispose it? – Thin 2/10, 2015 at 13:46

That's actually a good point. So far I have always managed to structure the code such that I was only calling that function once. I think that's good style in any case. – Flu 2/10, 2015 at 13:52

A short answer is that you don't have to dispose it although it is a good exercise to dispose any IDisposable objects that you own.

In fact, trying to dispose WebException.Response or the stream returned from it would cause problems you mentioned since you may encounter code attempting to read its properties in upper call chain's exception handler.

The reason why it is safe not to dispose it is because HttpWebRequest internally makes a memory stream from the network stream before it throws WebException, and the underlying network stream has already been closed/disposed. So it doesn't really have any unmanaged resource associated at that point. This I assume was a decision to make it easier to handle the exception.

Unfortunately MSDN documentation doesn't have any explanation about this behavior. Technically the implementation can change in future causing problem to your code not disposing the HttpWebResponse and/or associated stream you get from WebException, but it is highly unlikely that this behavior would change the implementation given many applications depend on current behavior.

I have to add though that it is a good practice that you should dispose IDisposable objects you own. If possible, use HttpClient class instead so that you don't have to deal with this situation at all. If you can't, consider handling the WebException yourself and throw new type of exception that does not expose WebException to the caller of your code so that you don't run into a situation where a caller is trying to access WebException.Response after you dispose it.

Disclaimer: I work for Microsoft, but this does not represent my employer or .NET Framework team's view. No warranty implied.

Bucella answered 31/3, 2017 at 2:3 Comment(0)