Search code examples
c#streamdispose

Disposing a StreamReader that read a stream that is defined out of scope?


In an utility method, which accepts a Stream parameter, I rely on some StreamReader to analyse data.

I don't want to close the incoming stream in my method. I want to let the caller method to take the decision to dispose the stream.

Is it safe to not dispose the opened StreamReader? I mean, will it eventually be automatically disposed? Will it lead to memory leaks?

Here is my utility method. Its goal is to read a Stream, and return its content as a string, regardless of how the data is encoded:

    public static string GetStringAutoDetectEncoding(Stream data, out Encoding actualEncoding)
    {
        // 1. Is there a Bye Order Mask ?
        var candidateEncoding = DetectEncodingWithByteOrderMask(data);

        // 2a. No BOM, the data is either UTF8 no BOM or ANSI
        if (candidateEncoding == Encoding.Default)
        {
            var utf8NoBomEncoding = Encoding.GetEncoding("utf-8",new EncoderExceptionFallback(), new DecoderExceptionFallback());

            var positionBackup = data.Position;
            var sr = new StreamReader(data, utf8NoBomEncoding);
            try
            {
                // 3. Try as UTF8 With no BOM
                var result = sr.ReadToEnd(); // will throw error if not UTF8
                actualEncoding = utf8NoBomEncoding; // Probably an UTF8 no bom string
                return result;
            }
            catch (DecoderFallbackException)
            {
                // 4. Rewind the stream and fallback to ASNI
                data.Position = positionBackup;
                var srFallback = new StreamReader(data, candidateEncoding);
                actualEncoding = candidateEncoding;
                return srFallback.ReadToEnd(); ;
            }
        }
        // 2b. There is a BOM. Use the detected encoding
        else
        {
            var sr = new StreamReader(data, candidateEncoding);
            actualEncoding = candidateEncoding;
            return sr.ReadToEnd(); ;
        }
    }

Then, I can have some methods in the like this:

void Foo(){
    using(var stream = File.OpenRead(@"c:\somefile")) {
        Encoding detected;
        var fileContent = MyUtilityClass.GetStringAutoDetectEncoding(stream, detected);

        Console.WriteLine("Detected encoding: {0}", encoding);
        Console.WriteLine("File content: {0}", fileContent);
    }
}

Solution

  • You could invert control using a closure. That is, create a method like so:

    // This method will open the stream, execute the streamClosure, and then close the stream.
    public static String StreamWork(Func<Stream, String> streamClosure) {
        // Set up the stream here.
        using (Stream stream = new MemoryStream()) { // Pretend the MemoryStream is your actual stream.
    
            // Execute the closure. Return it's results.
            return streamClosure(stream);
        }
    }
    

    which is responsible for opening / closing the stream within the method.

    Then you simply wrap up all the code that needs the stream into a Func<Stream, String> closure, and pass it in. The StreamWork method will open the stream, execute your code, then close the stream.

    public static void Main()
    {
        // Wrap all of the work that needs to be done in a closure.
        // This represents all the work that needs to be done while the stream is open.
        Func<Stream, String> streamClosure = delegate(Stream stream) {
            using (StreamReader streamReader = new StreamReader(stream)) {
                return streamReader.ReadToEnd();
            }
        };
    
        // Call StreamWork. This method handles creating/closing the stream.
        String result = StreamWork(streamClosure);
        Console.WriteLine(result);
        Console.ReadLine();
    }
    

    UPDATE

    Of course, this method of inversion is a matter of preference as mentioned in the comments below. The key point is to ensure that the stream is closed rather than allowing it to float around until the GC cleans it up (since the whole point of having stuff implement IDisposable is to avoid that sort of situation to begin with). Since this is a library function that accepts a Stream as input, the assumption is that the method-consumer will be creating the stream, and therefore as you point out, has the responsibility of ultimately closing the stream as well. But for sensitive resources where you are concerned about ensuring clean up occurs absolutely, inversion is sometimes a useful technique.