How to return a Stream from a method, knowing it s

Question

I have a method that takes FileStream as input. This method is running inside a for loop.

private void UploadFile(FileStream fileStream)
{
    var stream = GetFileStream();
    // do things with stream
}

I have another method which creates and returns the FileStream:

private FileStream GetFileStream()
{
    using(FileStream fileStream = File.Open(myFile, FileMode.Open))
    {
        //Do something
        return fileStream;
    }
}

Now the first method throws an ObjectDisposedException when I try to access the returned FileStream, probably because it is already closed since I am using "using" to properly dispose the stream.

If I don't use "using" and instead use it as follows, then the FileStream remains open and the next iteration of the loop (operating on the same file) throws an exception telling the file is already in use:

private FileStream GetFileStream()
{
    FileStream fileStream = File.Open(myFile, FileMode.Open);
    //Do something
    return fileStream;
}

If I use a try-finally block, where I close the stream in the finally then it also throws the ObjectDisposedException.

How to effectively return file stream and close it?

Answer 1

If you have a method that need to return an open file stream then all callers of that method need to take responsibility for disposing of the returned stream, since it cannot dispose of the stream before returning it.

Answer 2

The problem is that the FileStream object is disposed as soon as you exit from the GetFileStream() method, leaving it in an unusable state. As other answers already indicate, you need to remove the using block from that method and instead put the using block around any code that calls this method:

private FileStream GetFileStream()
{
    FileStream fileStream = File.Open(myFile, FileMode.Open);
    //Do something
    return fileStream;
}

using (var stream = GetFileStream())
{
    UploadFile(stream);
}

However, I want to take this a step further. You want a way to protect the stream created by your GetFileStream() from the case where a sloppy programmer might call the method without a using block, or at least somehow strongly indicate to callers that the result of this method needs to be enclosed with a using block. Therefore, I recommend this:

public class FileIO : IDisposable
{
    private FileStream streamResult = null;

    public FileStream CreateFileStream(string myFile)
    {
        streamResult = File.Open(myFile, FileMode.Open);
        //Do something
        return streamResult;
    }

    public void Dispose()
    { 
       if (streamResult != null) streamResult.Dispose();         
    }

}

using (var io = FileIO())
{
    var stream = io.CreateFileStream(myFile);

    // loop goes here.
}

Note that you don't necessarily need to create a whole new class for this. You may already have an appropriate class for this method where you can just add the IDisposable code. The main thing is that you can use IDisposable as a signal to other programmers that this code should be wrapped with a using block.

Additionally, this sets you up to modify the class so that you could create your IDisposable object once, before the loop, and have the new class instance keep track of everything you need to dispose at the end of the loop.

Answer 3

When you return an IDisposable from a method, you are relegating the responsibility of disposing it to your caller. Thus, you need to declare your using block around the entire usage of the stream, which in your case presumably spans the UploadFile call.

using (var s = GetFileStream())
    UploadFile(s);