8

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.

Community
  • 1
  • 1
Peter
  • 3,322
  • 3
  • 27
  • 41
  • Possible duplicate of [How to properly dispose of a WebResponse instance?](https://stackoverflow.com/questions/1887314/how-to-properly-dispose-of-a-webresponse-instance) – Deantwo Feb 26 '18 at 08:10

2 Answers2

11

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.

Deantwo
  • 1,113
  • 11
  • 18
Sunghwa Jin
  • 151
  • 1
  • 3
3

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.

usr
  • 168,620
  • 35
  • 240
  • 369
  • 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. – Peter Oct 02 '15 at 07: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. – usr Oct 02 '15 at 10:01
  • 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). – usr Oct 02 '15 at 10:01
  • 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. – Peter Oct 02 '15 at 10:42
  • 1
    Double dispose is always safe in .NET by convention. Disposing an object does not delete it. – usr Oct 02 '15 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. – Peter Oct 02 '15 at 10:53
  • 1
    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. – usr Oct 02 '15 at 11:00
  • 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.) – Peter Oct 02 '15 at 11:04
  • 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. – usr Oct 02 '15 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. – Peter Oct 02 '15 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. – usr Oct 02 '15 at 13:13
  • I've personally experienced quite a few. Anyway, so you have no direct experience with disposing webexception.response? – Peter Oct 02 '15 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. – usr Oct 02 '15 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. – Peter Oct 02 '15 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. – usr Oct 02 '15 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. – Peter Oct 02 '15 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? – Peter Oct 02 '15 at 13:46
  • 1
    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. – usr Oct 02 '15 at 13:52