Is it safe to use 'using' instead of closing a WebResponse and StreamReader

Question

Currently

I've implemented a simple helper method for HttpWebRequest called GetResponse(url). Currently I'm manually closing the WebResponse and StreamReader after reading the result. I'm then returning said result like so:

// construct the request
HttpWebRequest request = (HttpWebRequest)WebRequest.Create(url);
request.Method = "GET";

// get the result
WebResponse response = request.GetResponse();
StreamReader reader = new StreamReader(response.GetResponseStream());
string result = reader.ReadToEnd();

// clean up and return the result
reader.Close();
response.Close();
return result;

Proposed

Is it safe to encompass the return within using statements instead of closing them; will this have the same effect as the .Close()es?

// construct the request
HttpWebRequest request = (HttpWebRequest)WebRequest.Create(url);
request.Method = "GET";

// get the result
using (WebResponse response = request.GetResponse())
{
    using (StreamReader reader = new StreamReader(response.GetResponseStream()))
    {
        return reader.ReadToEnd();
    }
}

Good question, because it certainly isn't totally safe for WCF https://msdn.microsoft.com/en-us/library/aa355056(v=vs.110).aspx when some exceptions require `.Abort()` to be called. — weston, Jul 10 '15 at 09:10

score 25 · Accepted Answer · answered Oct 12 '12 at 14:18

25

That's not only safe - it's safer than the original, in that it will dispose of the objects even if an exception is thrown; a using statement is equivalent to a try/finally statement.

In general, whenever you write a Close() or Dispose() call explicitly, consider whether you could use a using statement instead.

(Note that you're not using the encoding from the web response, by the way - you're always assuming UTF-8. Using WebClient instead can make this simpler, if that's an option.)

answered Oct 12 '12 at 14:18

Jon Skeet

1,421,763
867
9,128
9,194

Cheers Jon; I wasn't aware of the `WebClient` class, seems to offer what I've been trying to achieve. I'm going to move the request slightly higher up, so I can safely assume the response will be `application/json`. – Richard Oct 12 '12 at 15:10
Sorry I forgot to mention, it'll always be UTF-8. Thanks again. – Richard Oct 12 '12 at 15:45
+1 for "in general", because I do know of one such exception to the rule, when using a WCF client https://msdn.microsoft.com/en-us/library/aa355056(v=vs.110).aspx Annoying that things like that make you doubt yourself and have to look up whether to use `using` or not. – weston Jul 10 '15 at 09:14
DANGER. Nesting using statements can cause dispose to be called multiple times which is not guaranteed to be idempotent and can cause System.ObjectDisposedException. I like the clean asthetics of nested usings but after seeing the ObjectDisposedException noise it caused in dynatrace I heeded what I read here. https://msdn.microsoft.com/en-us/library/ms182334.aspx – HarryTuttle Dec 03 '15 at 00:45
1

@HarryTuttle: So what do you do if there's anything between the outer `using` statement and the inner one? Even ignoring asynchronous exceptions, that's surely a problem. I don't think I've ever seen this cause a problem - anywhere you *have* seen it causing a problem, I hope you've talked to whoever implemented `IDisposable` badly and asked them to fix it. As per MSDN: "A correctly implemented Dispose method can be called multiple times without throwing an exception." – Jon Skeet Dec 03 '15 at 07:10

score 1 · Answer 2 · edited Jul 28 '17 at 06:07

1

using (StreamReader reader = new StreamReader())
{
    // code
}

is the same as

StreamReader reader;
try
{
    reader = new StreamReader();
    // code
}
finally
{
    if (reader != null)
    {
        reader.Dispose();
    }
}

So it's nearly the same as your code, but safer because of the try/finally block.

edited Jul 28 '17 at 06:07

g t

7,287
7
50
85

answered Oct 12 '12 at 14:20

Dmitrii Dovgopolyi

6,231
2
27
44

1

You mean `finally` instead of `catch`. It runs the dispose code even if there is no exception. – Chris Oct 12 '12 at 14:31

score 1 · Answer 3 · answered Oct 12 '12 at 14:26

I would suggest do this:

    string ret = string.Empty;
    using (WebResponse response = request.GetResponse())
    {
        using (StreamReader reader = new StreamReader(response.GetResponseStream()))
        {
            ret = reader.ReadToEnd();
        }
    }
    return ret;

it is safe to use "using", it will dispose the WebResponse and StreamReader, but it does not guarantee it will run the return.

Is it safe to use 'using' instead of closing a WebResponse and StreamReader

Currently

Proposed

3 Answers3