views:

327

answers:

4

Normally, one writes code something like this to download some data using a WebRequest.

using(WebResponse resp = request.GetResponse())  // WebRequest request...
   using(Stream str = resp.GetResponseStream())  
      ; // do something with the stream str

Now if a WebException is thrown, the WebException has a reference to the WebResponse object, which may or may not have Dispose called (depending on where the exception has happened, or how the response class is implemented) - I don't know.

My question is how one is supposed to deal with this. Is one supposed to be coding very defensively, and dispose of the response in the WebException object (that would be a little weird, as WebException is not IDisposable). Or is one supposed to ignore this, potentially accessing a disposed object or never disposing an IDisposable object? The example given in the MSDN documentation for WebException.Response is wholly inadequate.

+2  A: 

I'm pretty sure that when you have a using statement the object is disposed, regardless of how you exit the using block (Be it via exception, return or simply progressing through the function).

I suspect you'll find that the object inside the WebException has already been disposed if you let it leave the using block.

Remember, disposing of an object doesn't necessarily prevent accessing it later. It can be unpredictable to try to call methods on it later, causing exceptions of it's own or very weird behavior (And hence I wouldn't recommend it). But even still a large portion of the object is still left behind for the garbage collector even if you dispose it, and hence is still accessible. The purpose of the dispose is typically to clean up resource handles (Like in this case active TCP connections) that for performance reasons you can't realistically leave laying around until the garbage collector finds them. I only mention this to clarify that it's not mutually exclusive for it to be disposed and the exception to hold a reference to it.

Tim Schneider
Indeed. `using` is syntatic sugar for try { } finally { }, and the object will always be disposed in the finally block.
Jeremy McGee
How about an exception happening in GetResponse? The result will not have been assigned to the resp variable, thus returned from the expression, thus disposed by the using statement.I understand one can access a disposed object, however, one is not really supposed to. If this is the intention of this api, isn't the api "badly" designed?
Marcus
No, if GetResponse throws no object will be disposed. But you can't know if an object has been created since only the internals of GetResponse will know that, so it's GetResponse's responsibility to make sure there is no leak in this situation.
erikkallen
As an example, just to illustrate my question WebRequest test = WebRequest.Create("http://foo.bar"); try { using(WebResponse resp = test.GetResponse()) ; } catch(WebException e) { Console.WriteLine(e.Response == null); }This writes "False" to the console. Clearly I did not dispose the response object here, so should I?I understand there is probably no leak, but that is not really the point here.
Marcus
Try this then: WebRequest test = WebRequest.Create("http://www.google.com"); WebResponse resp = test.GetResponse(); resp.Dispose(); Console.WriteLine(resp == null); It'll also write false. My point was that just cause something is not null doesn't mean it hasn't been disposed (In fact calling dispose doesn't make any type that I know of null).
Tim Schneider
A: 

A very interesting question (although worth pointing out that the WebResponse object will have been disposed as you exit the using). My gut feeling is that it doesn't really matter that you've got a reference to this disposed WebResponse object as long as you don't try to do anything "operational" with it.

You can probably still access certain properties on the instance for logging purposes (such as the ResponseUri) without getting an ObjectDisposedException, but overall reference held by the exception is not there so you can continue using the instance.

I'd be interested to see what others say.

Rob Levine
A: 
using (var x = GetObject()) {
     statements;
}

is (almost) equivalent to

var x = GetObject();
try {
    statements;
}
finally {
     ((IDisposable)x).Dispose();
}

so your object will always be disposed.

This means that in your case

try {
    using (WebResponse resp = request.GetResponse()) {
        something;
    }
}
catch (WebException ex) {
    DoSomething(ex.Response);
}

ex.Response will be the same object as your local resp object, which is disposed when you get to the catch handler. This means that DoSomething is using a disposed object, and will likely fail with an ObjectDisposedException.

erikkallen
+1  A: 

I have had a quick peek with Reflector, and can now say:

  • WebResponse, being an abstract class, delegates all its closing/disposing behaviour to its derived classes.
  • HttpWebResponse, being the derived class you are almost certainly using here, in its close/dispose methods, is only concerned with disposing the actual response stream. The rest of the class state can be left to the GC's tender mercies.

It follows that it's probably safe to do whatever you like with regard to exception handling, as long as:

  • When you read the response stream from WebResponse in the try block, enclose it in a using block.
  • If you read the response stream from WebException in the catch block, enclose it in a using block as well.
  • There is no need to worry about disposing of WebException itself.
Christian Hayter