When to call WebResponse.Close()

Question

WebResponse response;
try
{                
 HttpWebRequest request = (HttpWebRequest)WebRequest.Create(url);
 request.Timeout = 20000;
 response = request.GetResponse();

 request = (HttpWebRequest)WebRequest.Create(url2);
 response = request.GetResponse();
}
catch(Exception ex)
{
 //do something
}              
finally
{
}

where should response.Close() be called?

after every GetResponse() in try?
after last GetResponse() in try - once?
in finally block?

John Saunders · Accepted Answer · 2011-04-29T21:40:18.650

26

None of the above. You should be using a using block:

HttpWebRequest request = (HttpWebRequest)WebRequest.Create(url);
request.Timeout = 20000;
using (WebResponse response = request.GetResponse())
{
    using (var stream = response.GetResponseStream())
    {
        using (var reader = new StreamReader(stream))
        {
            var result = reader.ReadToEnd();
            // Do something with result
        }
    }
}

A using block will ensure that the Dispose method is called, whether or not there is an exception. Dispose will do the same thing as Close.

using (var d = new DisposableClass()){code;}

is equivalent to:

DisposableClass d = null;
try
{
    d = new DisposableClass();
    code;
}
finally
{
    if (d != null)
        ((IDisposable)d).Dispose();
}

edited Apr 29 '11 at 21:40

answered Jul 26 '09 at 12:27

John Saunders

160,644
26
247
397

1

For a more detailed explanation, you could show how all those using statements get converted to try/finally statements :) Only reason I say this is because he asked if he should put it in a finally statement, which you are in a sense doing... Obviously in a cleaner / easier to read manner. – Allen Rice Jul 27 '09 at 21:00
Surely this must be doable without using 'using', just the standard try catch finally blocks? – UpTheCreek Apr 29 '11 at 21:10
Is it mandatory to dispose the WebResponse instance? I don't see dispose in Vs2008's intellisense. – Michel van Engelen Jul 14 '11 at 10:37
Hmm when I use ILSpy (instead of .$$$ Reflector) I see an IDisposable interface. The only thing dispose does is run Close(). – Michel van Engelen Jul 14 '11 at 11:21
@Michel: so does that answer your question? My answer is: if it implements IDisposable, and if you created it, then you need to Dispose of it. – John Saunders Jul 14 '11 at 13:39
@Michel: I don't know. Were you using VB.NET? By default it doesn't show all details. – John Saunders Jul 14 '11 at 14:45
This appears to have problems. If you kill the process while the request is being made, it will come back with a user-unhandled exception for ObjectDisposedException - Cannot access a disposed object. in Socket.cs bool SendToAsync() – Latency Aug 03 '18 at 00:16

score 1 · Answer 2 · answered Jul 26 '09 at 12:24

1

Put it in the finally block. As per MSDN:

The finally block is useful for cleaning up any resources allocated in the try block as well as running any code that must execute even if there is an exception. Control is always passed to the finally block regardless of how the try block exits.

answered Jul 26 '09 at 12:24

marco0009

163
2
8

But if he puts response.Close() in finally he will get a 'use of unasigned variable' error. – UpTheCreek Apr 29 '11 at 21:08

score 0 · Answer 3 · answered Jul 26 '09 at 13:34

0

Note that nested using blocks don't need curly braces, improving readability. So John Saunder's code could be written:

HttpWebRequest request = (HttpWebRequest)WebRequest.Create(url);
request.Timeout = 20000;
using (WebResponse response = request.GetResponse())
using (var stream = response.GetResponseStream())
using (var reader = new StreamReader(stream))
{
    var result = reader.ReadToEnd();
    // Do something with result
}

VS.NET understands that such nested blocks don't need indenting. Note btw that if you know the encoding of the response or are going to ignore it anyhow, WebClient provides a simpler API - missing header information, so Header-based (transfer/text) encoding detection becomes impossible, but otherwise it works fine.

answered Jul 26 '09 at 13:34

Eamon Nerbonne

47,023
20
101
166

1

I would argue that this reduces readability. I almost always add braces, even on single-line if statements, except for rare cases like "if (simple-condition) return;" – John Saunders Jul 26 '09 at 13:39
1

For short bits of code maybe - for real files adding these unnecessary braces on redundant lines means that your code becomes much longer and less fits on one screen, meaning you'll have less overview. The real hint anyhow should be indentation, and VS.NET automatically indents such code in a non-confusing fashion. – Eamon Nerbonne Jul 27 '09 at 06:47

score -1 · Answer 4 · answered Jul 26 '09 at 12:26

-1

I would suggest the below

        try
        {
            HttpWebRequest request = (HttpWebRequest)WebRequest.Create("http://www.google.com");
            request.Timeout = 20000;
            using (var response = request.GetResponse())
            {
                //Do something with response.
            }


            request = (HttpWebRequest)WebRequest.Create("http://www.bing.com");
            using (var response = request.GetResponse())
            {
                //Do somehing with response
            }
        }
        catch (Exception ex)
        {
            //do something
        }
        finally
        {
        }

answered Jul 26 '09 at 12:26

Ramesh

13,043
3
52
88

1

Surely you need to dispose/Close the first "request" instance, since you just overwrite that instance with a new one, you're at the mercy of the Garbage collector to when that first one gets disposed. – Marineio Jul 26 '09 at 13:37
2

WebRequest does not implement IDisposable. – John Saunders Jul 26 '09 at 13:41
There is no reason for a `finally` block here. Also, without having some actual code in the `catch`, this answer may leave the false impression that `try/catch` is required around all code. – John Saunders Apr 09 '15 at 19:52

When to call WebResponse.Close()

4 Answers4