how to fix Code Analysis warnings

Question

I have a code

public String makeHttpGetRequest(String url)
        {
            try
            {
                string responce = string.Empty;
                HttpWebRequest request = (HttpWebRequest)WebRequest.Create(url);
                request.AutomaticDecompression = DecompressionMethods.GZip;


                using (HttpWebResponse response = (HttpWebResponse)request.GetResponse())
                using (Stream stream = response.GetResponseStream())
                using (StreamReader reader = new StreamReader(stream))
                {
                    responce = reader.ReadToEnd();
                }
                return responce;
            }
            catch (Exception e)
            {
                Console.WriteLine("Internet Connection error" + e.Message);
                return null;
            }
        }

And i am getting a warning when i run code analysis in Visual studio that

CA2202 Do not dispose objects multiple times Object 'stream' can be disposed more than once in method 'InformationIO.makeHttpGetRequest(string)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 244 InformationIO.cs 244

line 224 refers to line 13 here the closing bracket before return responce;

How can i fix this warning.

score 1 · Answer 1 · answered Aug 23 '16 at 07:34

These two lines reference the same stream, and will try to dispose it twice:

using (Stream stream = response.GetResponseStream())
using (StreamReader reader = new StreamReader(stream))

Remove the second using block (of the three you have), as it's unnecessary in this case.

using (HttpWebResponse response = (HttpWebResponse)request.GetResponse())
{
    Stream stream = response.GetResponseStream());
    using (StreamReader reader = new StreamReader(stream))
    {
        responce = reader.ReadToEnd();
    }
}

If you want to be really sure the stream is disposed, add a finally block:

Stream stream = null;
try
{
    using (HttpWebResponse response = (HttpWebResponse)request.GetResponse())
    {
        stream = response.GetResponseStream());
        using (StreamReader reader = new StreamReader(stream))
        {
            responce = reader.ReadToEnd();
        }
    }
}
finally
{
    // check if stream is not null (although it should be), and dispose of it
    if (stream != null)
        stream.Dispose();
}

score 0 · Answer 2 · answered Aug 23 '16 at 07:32

From MSDN

The StreamReader object calls Dispose() on the provided Stream object when StreamReader.Dispose is called.

This means StreamReader will call Dispose on Stream object and your using statement will do so as well. Alternative could be:

using (HttpWebResponse response = (HttpWebResponse)request.GetResponse())
    using (StreamReader reader = new StreamReader(response.GetResponseStream()))
    {
       responce = reader.ReadToEnd();
    }
return responce;

score 0 · Answer 3 · answered Aug 23 '16 at 08:02

The deeper issue here is that the design of StreamReader is incorrect; it should never dispose the underlying stream because it doesn't own it.

The following pattern is pretty common:

public class Foo: IDisposable
{
    public Foo(Bar bar) {...}
}

public class Bar: IDisposable { ... }

When disposing foo, it should never dispose bar automatically for you, because it does not own bar; bar is an object supplied by the user of foo; its therefore responsability of the user to take care of bar.

This is essentially what is happening in your code; you are correctly taking care of stream but StreamReader is also taking care of something that is not really its business, disposing stream for you too.

So, to wrap up: Your code is correct. Due to a design flaw in StreamReader your correct code is creating an issue that needs to be taken care of; duplicate calls to Dispose().

Other answers point out good ways to fix the issue, so I wont duplicate code here.

how to fix Code Analysis warnings

3 Answers3