Should I call Close() or Dispose() for stream objects?

Question

Classes such as Stream, StreamReader, StreamWriter etc implements IDisposable interface. That means, we can call Dispose() method on objects of these classes. They've also defined a public method called Close(). Now that confuses me, as to what should I call once I'm done with objects? What if I call both?

My current code is this:

using (Stream responseStream = response.GetResponseStream())
{
   using (StreamReader reader = new StreamReader(responseStream))
   {
      using (StreamWriter writer = new StreamWriter(filename))
      {
         int chunkSize = 1024;
         while (!reader.EndOfStream)
         {
            char[] buffer = new char[chunkSize];
            int count = reader.Read(buffer, 0, chunkSize);
            if (count != 0)
            {
               writer.Write(buffer, 0, count);
            }
         }
         writer.Close();
      }
      reader.Close();
   }
}

As you see, I've written using() constructs, which automatically call Dispose() method on each object. But I also call Close() methods. Is it right?

Please suggest me the best practices when using stream objects. :-)

MSDN example doesn't use using() constructs, and call Close() method:

• How to: Download Files with FTP

Is it good?

Conflagrant

If yo're using ReSharper you could define this as a "antipattern" within the patter catalog. ReSharper will mark each usage as error/hint/warning regarding to your definition. It's also possible to define how ReSharper has to apply a QuickFix for such an occurrence. – Jadda

Just a tip: You can use the using statement like that for multiple disposable itens: using (Stream responseStream = response.GetResponseStream()) using (StreamReader reader = new StreamReader(responseStream)) using (StreamWriter writer = new StreamWriter(filename)) { //...Some code } – Liris

possible duplicate of Does Stream.Dispose always call Stream.Close (and Stream.Flush) – Meridith

You don't need to nest your using statements like that you can stack them on top of one another and have one set of brackets. On another post, I suggested an edit for a code snippet that should have had using statements with that technique if you'd like to look and fix your "code arrow": #5283499 – Ericerica

@TimothyGonzalez You do have to nest your using statements like that. using permits only one type, even if you are initializing multiple resources in the same statement. If you are using multiple statements or multiple types, by definition you must nest using statements; here, the objects are different types and must be in separate using statements. – Gio

@Gio You can have multiple using statements, but not nest them and instead stack them. I don't mean syntax like this which restricts the type: using (MemoryStream ms1 = new MemoryStream(), ms2 = new MemoryStream()) { }. I mean like this where you can redefine the type: using (MemoryStream ms = new MemoryStream()) using (FileStream fs = File.OpenRead("c:\\file.txt")) { } – Ericerica

@TimothyGonzalez Sorry to be pedantic but those latter statements are nested. – Gio

Answer 1

A quick jump into Reflector.NET shows that the Close() method on StreamWriter is:

public override void Close()
{
    this.Dispose(true);
    GC.SuppressFinalize(this);
}

And StreamReader is:

public override void Close()
{
    this.Dispose(true);
}

The Dispose(bool disposing) override in StreamReader is:

protected override void Dispose(bool disposing)
{
    try
    {
        if ((this.Closable && disposing) && (this.stream != null))
        {
            this.stream.Close();
        }
    }
    finally
    {
        if (this.Closable && (this.stream != null))
        {
            this.stream = null;
            /* deleted for brevity */
            base.Dispose(disposing);
        }
    }
}

The StreamWriter method is similar.

So, reading the code it is clear that that you can call Close() & Dispose() on streams as often as you like and in any order. It won't change the behaviour in any way.

So it comes down to whether or not it is more readable to use Dispose(), Close() and/or using ( ... ) { ... }.

My personal preference is that using ( ... ) { ... } should always be used when possible as it helps you to "not run with scissors".

But, while this helps correctness, it does reduce readability. In C# we already have plethora of closing curly braces so how do we know which one actually performs the close on the stream?

So I think it is best to do this:

using (var stream = ...)
{
    /* code */

    stream.Close();
}

It doesn't affect the behaviour of the code, but it does aid readability.

Thriftless

"In C# we already have plethora of closing curly braces so how do we know which one actually performs the close on the stream?" I don't think that this is a big problem: The stream is closed "at the right time", i.e., when the variable goes out of scope and is no longer needed. – Turnpike

@Turnpike - I agree that the stream is closed at the right time. My position is that readability of the code is improved by including the Close and including the Close is benign as it doesn't change the behaviour. – Thriftless

Hmm, no, that is a "why the heck is he closing it twice??" speed bump while reading. – Bloodshot

@Enigmativity: Good point. (+1 for the good and detailed answer, BTW) – Turnpike

@HansPassant - I'd look at it from the other POV. You're obviously an experienced developer so you know that using effectively closes the stream so it seems like I'm closing it twice - but again you're experienced so you know that it doesn't matter if I do. However, if someone less experienced looks at the code they might think it wasn't closed unless Close was explicitly called. This pattern makes coding safer for inexperienced developers and just gives experienced developers a "chuckle". :-) – Thriftless

I disagree with the redundant Close() call. If someone less experienced looks at the code and doesn't know about using he will: 1) look it up and learn, or 2) blindly add a Close() manually. If he picks 2), maybe some other developer will see the redundant Close() and instead of "chuckling", instruct the less experienced developer. I'm not in favor of making life difficult for inexperienced developers, but I'm in favor of turning them into experienced developers. – Deteriorate

@R.MartinhoFernandes - I'm an experienced developer and I would instruct less experienced developers to include the redundant close for the reasons I have given. I do think this discussion is boiling down to a simple "matter of opinion" rather than what is correct/best or not. It's good that opinions have been aired though. Cheers. – Thriftless

If you use using + Close() and turn /analyze on, you get "warning : CA2202 : Microsoft.Usage : Object 'f' can be disposed more than once in method 'Foo(string)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.: Lines: 41" So while the current implementation is fine with calling Close and Dispose, according to documentation and /analyze, it's not ok and might change in future versions of .net. – Raine

@Raine - Interesting info. Much appreciated. – Thriftless

+1 for the good answer. Another thing to consider. Why not add a comment after the closing brace like //Close or as I do, being a newbie, I add a one liner after any closing brace thats not clear. like for example in a long class I would add //End Namespace XXX after the final closing brace, and //End Class YYY after the second final closing brace. Is this not what comments are for. Just curious. :) As a newbie, I saw such code, hense why I came here. I did ask the question "Why the need for the second close". I feel extra lines of code dont add to clarity. Sorry. – Consistence

I like my code to say what it is doing. So, I'd rather use the .Dispose() explicitly.Using is optional anyway and, IMHO, it may or may not be convenient to use, according to circumstances and personal preference. Using it, adds one more block to your code which is unneeded if you explicitly dispose your objects. – Overweary

@Overweary - I'd rather the extra block than forgetting to cal Dispose. I think the using keyword is explicit enough. Also using automatically adds in a try/catch that ensures that the dispose is called. It actually changes the semantics for the better, I think. – Thriftless

While I appreciate the details of the answer, I really don't like relying on implementation details of 3rd party functionality for making a decision on the "proper" way to do things. If Microsoft decides to change the implementation so it does matter whether you call Close or Dispose (and in the right order) then whose fault is it when your application breaks because you are relying on implementation details and not documentation details? – Ellerd

@Ellerd - I don't think my suggested coding style would be affected by a change from Microsoft. I don't think that they'd change this behaviour now in any case. – Thriftless

I wasn't specifically criticizing your example. I really liked your answer, it was informative. I actually think Microsoft wrote it this way to avoid confusion, so I agree they won't change it. However, I was more intending to make a general comment that relying on a current 3rd party implementation is a dangerous practice. It can change at any time and finding the cause could be really, really hard. – Ellerd

@Ellerd - I agree. Cheers. – Thriftless

Calling Close() inside a using block clutters your code and makes it confusing for everyone -- either they'll think the extra close is necessary or they'll be wondering why in the world someone added a redundant line. The documentation itself states "Instead of calling this method, ensure that the stream is properly disposed." – Elidaelidad

Using doesn't help if the Stream isn't confined to a local scope. The below answers correctly say that .Dispose should be used as .Close is an outdated method that was created before IDisposable. – Caudillo

@marc40000: Calling Dispose multiple times is explicitly permitted. That code analysis is incorrect (you can't get ObjectDisposedException from Dispose(), not from any class following the IDisposable contract). Not saying it shouldn't be flagged at all -- the programmer probably didn't intend multiple dispose, after all -- but the instruction that comes with it is 100% incorrect. learn.microsoft.com/en-us/dotnet/api/system.idisposable.dispose – Livestock

"If an object's Dispose method is called more than once, the object must ignore all calls after the first one. The object must not throw an exception if its Dispose method is called multiple times. Instance methods other than Dispose can throw an ObjectDisposedException when resources are already disposed." – Livestock

Answer 2

No, you shouldn't call those methods manually. At the end of the using block the Dispose() method is automatically called which will take care to free unmanaged resources (at least for standard .NET BCL classes such as streams, readers/writers, ...). So you could also write your code like this:

using (Stream responseStream = response.GetResponseStream())
    using (StreamReader reader = new StreamReader(responseStream))
        using (StreamWriter writer = new StreamWriter(filename))
        {
            int chunkSize = 1024;
            while (!reader.EndOfStream)
            {
                 char[] buffer = new char[chunkSize];
                 int count = reader.Read(buffer, 0, chunkSize);
                 if (count != 0)
                 {
                     writer.Write(buffer, 0, count);
                 }
            }
         }

The Close() method calls Dispose().

Superficies

I'm pretty sure you don't need to be using the first responseStream since that is wrapped by the reader which will make sure its closed when the reader is disposed. +1 nontheless – Kodok

This is confusing when you said The Close method calls Dispose... and in the rest of your post, you're implying that Dispose() would call Close(), I shouldn't call the latter manually. Are you saying they call each other? – Conflagrant

@Nawaz, my post was confusing. The Close method simply calls Dispose. In your case you need Dispose in order to free unmanaged resources. By wrapping your code in using statement the Dispose method is called. – Superficies

Terrible answer. It assumes you can use a using block. I'm implementing a class that writes from time to time and therefore cannot. – Mopup

@Mopup Your class should then implement the IDisposable interface, and possibly also Close() if close is standard terminology in the area, so that classes using your class can use using (or, again, go for the Dispose Pattern). – Twickenham

The OP asked about properly closing stream objects. Not about some syntactic sugar. – Overlarge

using doesn't help if the Stream isn't confined to a local scope. The below answers correctly say that .Dispose should be used as .Close is an outdated method that was created before IDisposable. – Caudillo

Answer 3

For what it's worth, the source code for Stream.Close explains why there are two methods:

// Stream used to require that all cleanup logic went into Close(),
// which was thought up before we invented IDisposable.  However, we
// need to follow the IDisposable pattern so that users can write
// sensible subclasses without needing to inspect all their base
// classes, and without worrying about version brittleness, from a
// base class switching to the Dispose pattern.  We're moving
// Stream to the Dispose(bool) pattern - that's where all subclasses
// should put their cleanup now.

In short, Close is only there because it predates Dispose, and it can't be deleted for compatibility reasons.

Polemic

Answer 4

The documentation says that these two methods are equivalent:

StreamReader.Close: This implementation of Close calls the Dispose method passing a true value.

StreamWriter.Close: This implementation of Close calls the Dispose method passing a true value.

Stream.Close: This method calls Dispose, specifying true to release all resources.

So, both of these are equally valid:

/* Option 1, implicitly calling Dispose */
using (StreamWriter writer = new StreamWriter(filename)) { 
   // do something
} 

/* Option 2, explicitly calling Close */
StreamWriter writer = new StreamWriter(filename)
try {
    // do something
}
finally {
    writer.Close();
}

Personally, I would stick with the first option, since it contains less "noise".

Turnpike

Answer 5

This is an old question, but you can now(C# 8.0) write using statements without needing to block each one. They will be disposed of in reverse order when the containing block is finished.

using var responseStream = response.GetResponseStream();
using var reader = new StreamReader(responseStream);
using var writer = new StreamWriter(filename);

int chunkSize = 1024;

while (!reader.EndOfStream)
{
    char[] buffer = new char[chunkSize];
    int count = reader.Read(buffer, 0, chunkSize);
    if (count != 0)
    {
        writer.Write(buffer, 0, count);
    }
}

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/using

Microbiology

Wow. Thanks for the warning! Talk about confusing. It was enough of an exercise looking up which block-end brace disposed the stream. Now we can guess if it can remain open until the end of the function? What were they thinking? Microsoft developers: creating sharp edges since the 20th century! – Gio

Heartily disagree. In the majority of cases this vastly improves readability (less indentation) and the disposal at the end of the containing scope remains deterministic. If you need disposal at a different point, using () {} is still there for you. – Tact

Answer 6

On many classes which support both Close() and Dispose() methods, the two calls would be equivalent. On some classes, however, it is possible to re-open an object which has been closed. Some such classes may keep some resources alive after a Close, in order to permit reopening; others may not keep any resources alive on Close(), but might set a flag on Dispose() to explicitly forbid re-opening.

The contract for IDisposable.Dispose explicitly requires that calling it on an object which will never be used again will be at worst harmless, so I would recommend calling either IDisposable.Dispose or a method called Dispose() on every IDisposable object, whether or not one also calls Close().

Nittygritty

FYI here's an article on the MSDN blogs that explains the Close and Dispose fun. blogs.msdn.com/b/kimhamil/archive/2008/03/15/… – Dorey

Answer 7

Just to complement other answers, as of C# 8.0, you don't need to open a block of code just to use an using statement

if (...) 
{ 
   using FileStream f = new FileStream(@"C:\users\jaredpar\using.md");
   // statements
}

// Equivalent to 
if (...) 
{ 
   using (FileStream f = new FileStream(@"C:\users\jaredpar\using.md")) 
   {
    // statements
   }
}

docs:

https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-8.0/using

Sculpturesque