CA2000/CA2202 for Stream in a using statement

Question

CA2000 and CA2202 warnings have recently been the bane of my existence. What am I doing wrong here? I basically get a FileStream using File.Open and then pass it into a function that may return a new stream or may return the same stream. I then perform some more actions on my stream and then in my finally block I dispose the stream I was using if it was different.

I get two CA warnings. 2000 for fileStream in the using block and 2202 for changedStream in the finally block. What gives?

using (Stream fileStream = File.Open(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
{
    Stream changedStream = null;
    try
    {
        changedStream = someCondition ? fileStream : DoSomeActionThatMayReturnNewStream(fileStream);
        DoSomeMoreStuffWithStream(changedStream);
    }
    finally
    {
        if (changedStream != null && changedStream != fileStream)
        {
            changedStream.Dispose();
        }
    }
}

score 1 · Answer 1 · answered Feb 19 '14 at 19:03

Under what cases, if any, will DoSomeActionThatMayReturnNewStream dispose of the passed-in stream? If when it creates a new stream it disposes of the passed-in one (which would generally be expected), the Dispose triggered by the using block will be redundant.

It appears as though the behavior of your code might be correct if DoSomeActionThatMayReturnNewStream never disposes of the passed-in stream, but FxCop has no way of analyzing its complex and unorthodox pattern of object ownership. I would suggest that it would be better to do something like

Stream inputFile = null;
try
{
  inputFile = File.Open(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite);
  DoSomeActionThatMayReturnNewStream(ref inputFile);
  DoSomeMoreStuffWithStream(inputFile);
}
finally
{
  if (inputFile != null)
    inputFile.Dispose();
}

The DoSomeActionThatMayReturnNewStream should dispose of the old stream if it's going to open a new one. It should null the variable immediately before closing the old stream and and assign it immediately upon opening the new one. That will ensure that if an exception occurs during the method, the old stream will get disposed if and only if it hasn't been disposed before, and the new stream will get disposed if its constructor completed, even if the DoSomeActionThatMayReturnNewStream threw an exception after that [if that method calls Dispose on the new stream in case an exception gets thrown, it should null out the variable in such case].

This would be perfect if I had control of the API `DoSomeActionThatMayReturnNewStream`. It does actually go ahead and call `Stream.Close` any time it needs to create a new stream. But since it is not a ref parameter, there is no way to signal to the caller that it did so. — sohum, Feb 19 '14 at 20:48
Also, does FxCop automatically assume that whenever we pass a stream as a parameter to a function that it will be disposed/not disposed or does it actually analyze the function we pass it into? — sohum, Feb 19 '14 at 20:50
@sohum: By my understanding, FxCop knows that a few methods take ownership of a passed-in parameter, but cannot generally infer such things. If you had control of the API, using a `ref` parameter would allow FxCop to be agnostic as to the object's ownership. Otherwise, the best one can do is probably use the same variable for the original object and the new one, and figure that if the call succeeds the code will have responsibility for disposing the object in the variable regardless of whether it's the original one or the new one. One won't be able to employ `using`, but... — supercat, Feb 19 '14 at 21:39
...it's not really buying anything here anyhow, since your code will only be holding one resource at a time (even though one may have acquired it two different ways), one needs manual disposal for one of the acquisition cases, and that same disposal could handle both. — supercat, Feb 19 '14 at 21:40

score 0 · Answer 2 · answered Feb 19 '14 at 19:26

0

What's wrong with the following:

using (var fileStream = System.IO.File.Open(path, FileMode.Open, FileAccess.Read, FileShare.ReadWrite))
{
    using (var changedStream = someCondition ? fileStream : DoSomeActionThatMayReturnNewStream(fileStream))
    {
        DoSomeMoreStuffWithStream(changedStream);
    }
}

answered Feb 19 '14 at 19:26

John Saunders

160,644
26
247
397

Wouldn't this dispose fileStream twice if changedStream == fileStream? Once in each using block? – sohum Feb 19 '14 at 21:02
You're right. I think you'll need `if (someCondition){DoSomeMoreStuffWithStream(fileStream);}else{using (var changedStream = DoSomeActionThatMayReturnNewStream(fileStream)){DoSomeMoreStuffWithStream(changedStream);}` – John Saunders Feb 19 '14 at 22:59

CA2000/CA2202 for Stream in a using statement

2 Answers2