CODEWITHSUNDEEP
c#dispose
Chris Conway
Chris Conway

Reputation: 16519

Dispose of object more than one time error. CA2202. Is there a better way?

How can I ensure the following code is disposing of all objects in a better fashion? Currently, Code Analysis is telling me

Error 45 CA2202 : Microsoft.Usage : Object 'ns' can be disposed more than once in method 'CPCommunicator.GetResults(string)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 64, 65

NetworkStream ns = null;
StreamWriter requestStream = null;
TextReader responseStream = null;

var results = new StringBuilder();

try
{
    ns = new NetworkStream(CreateConnection(), true);
    requestStream = new StreamWriter(ns);
    requestStream.Flush();
    responseStream = new StreamReader(ns);

    requestStream.Write(reportData);
    requestStream.Flush();
    while (responseStream.Peek() != -1)
    {
        var currentLine = responseStream.ReadLine();
        results.Append(currentLine);
        results.Append("\n");
    }
}
finally
{
    if (requestStream != null) requestStream.Close();
    if (responseStream != null) responseStream.Close();
    if (cpNetworkStream != null) cpNetworkStream.Close();
}

Since both requestStream and responseStream use ns, they both dispose of ns so in order to satisfy the code analysis warning, I have to comment out the last two close methods in the finally block. But do I really want to do this?????

Upvotes: 4

Views: 3852

Answers (2)

Skywalker
Skywalker

Reputation: 412

I would refactor your code to be like this:

using (NetworkStream ns = new NetworkStream(CreateConnection(), true))
using (StreamWriter requestStream = new StreamWriter(ns))
using (TextReader responseStream = new StreamReader(ns))
{

    var results = new StringBuilder();

    requestStream.Flush();

    requestStream.Write(reportData);
    requestStream.Flush();
    while (responseStream.Peek() != -1)
    {
        var currentLine = responseStream.ReadLine();
        results.Append(currentLine);
        results.Append("\n");
    }
}

Upvotes: 2

Foxfire
Foxfire

Reputation: 5755

Yes, imho you really should only call it once.

Alternatively you could use the using syntax on ns, which makes the whole situation even clearer.

using (ns = new NetworkStream(CreateConnection(), true)) {
   ...
}

Upvotes: 4

Related Questions