26

A few days ago I posted some code like this:

StreamWriter writer = new StreamWriter(Response.OutputStream);
writer.WriteLine("col1,col2,col3");
writer.WriteLine("1,2,3");
writer.Close();
Response.End();

I was told that instead I should wrap StreamWriter in a using block in case of exceptions. Such a change would make it look like this:

using(StreamWriter writer = new StreamWriter(Response.OutputStream))
{
    writer.WriteLine("col1,col2,col3");
    writer.WriteLine("1,2,3");
    writer.Close(); //not necessary I think... end of using block should close writer
}
Response.End();

I am not sure why this is a valuable change. If an exception occurred without the using block, the writer and response would still be cleaned up, right? What does the using block gain me?

10 Answers 10

23

Nope the stream would stay open in the first example, since the error would negate the closing of it.

The using operator forces the calling of Dispose() which is supposed to clean the object up and close all open connections when it exits the block.

17

I'm going to give the dissenting opinion. The answer to the specific question "Is it necessary to wrap StreamWriter in a using block?" is actually No. In fact, you should not call Dispose on a StreamWriter, because its Dispose is badly designed and does the wrong thing.

The problem with StreamWriter is that, when you Dispose it, it Disposes the underlying stream. If you created the StreamWriter with a filename, and it created its own FileStream internally, then this behavior would be totally appropriate. But if, as here, you created the StreamWriter with an existing stream, then this behavior is absolutely The Wrong Thing(tm). But it does it anyway.

Code like this won't work:

var stream = new MemoryStream();
using (var writer = new StreamWriter(stream)) { ... }
stream.Position = 0;
using (var reader = new StreamReader(stream)) { ... }

because when the StreamWriter's using block Disposes the StreamWriter, that will in turn throw away the stream. So when you try to read from the stream, you get an ObjectDisposedException.

StreamWriter is a horrible violation of the "clean up your own mess" rule. It tries to clean up someone else's mess, whether they wanted it to or not.

(Imagine if you tried this in real life. Try explaining to the cops why you broke into someone else's house and started throwing all their stuff into the trash...)

For that reason, I consider StreamWriter (and StreamReader, which does the same thing) to be among the very few classes where "if it implements IDisposable, you should call Dispose" is wrong. Never call Dispose on a StreamWriter that was created on an existing stream. Call Flush() instead.

Then just make sure you clean up the Stream when you should. (As Joe pointed out, ASP.NET disposes the Response.OutputStream for you, so you don't need to worry about it here.)

Warning: if you don't Dispose the StreamWriter, then you do need to call Flush() when you're done writing. Otherwise you could have data still being buffered in memory that never makes it to the output stream.

My rule for StreamReader is, pretend it doesn't implement IDisposable. Just let it go when you're done.

My rule for StreamWriter is, call Flush where you otherwise would have called Dispose. (This means you have to use a try..finally instead of a using.)

5
  • "My rule for StreamReader is, pretend it doesn't implement IDisposable" - but not if you constructed the StreamReader using a filename, as it does own the stream in this case. I disagree that StreamReader/Writer is badly designed. While I understand the problems you describe, all design is trade-off and I think on balance the behavior is right for most use cases. Sure in your example above, you now have to nest your using statements, but it's not such a big deal. There's some discussion on responsibility for disposing in this question: stackoverflow.com/questions/674879
    – Joe
    Jun 19, 2009 at 20:47
  • @Joe: Your anti-rule is too strong. It should be, don't call Dispose (therefore don't implement a using block) on StreamReader/StreamWriter unless you intend to close the underlying stream. Maybe what's needed is something like the CloseInput flag on XmlReaderSettings, but for StreamReader instead. Jun 20, 2009 at 11:04
  • @Joe: True. What they should have done was, if you construct it with a filename, then the constructor creates a stream and Dispose closes it; but if you construct it with a stream, it doesn't close it, because somebody else owns it.
    – Joe White
    Jun 20, 2009 at 13:03
  • 1
    @John: So you like obtuse code? (grin) Come on. If you want to close the underlying Stream, then close the underlying Stream. Make your code say what you mean.
    – Joe White
    Jun 20, 2009 at 13:05
  • 13
    .Net 4.5 fixed this oversight, it now allows you to optionally leave the stream open: msdn.microsoft.com/en-us/library/gg712853.aspx
    – CubanX
    Oct 18, 2012 at 12:38
3

If an exception occurs without the using block and kills the program, you will be left with openconnections. The using block will always close the connection for you, similiar to if you were to use try{}catch{}finally{}

2
  • 1
    If the program exits, the handles will get cleaned up. The using statement only helps while the app is running. Jun 19, 2009 at 17:12
  • Finalizers are the term for what does the clean up on exit.
    – Guvante
    Jun 19, 2009 at 17:43
3

Eventually, the writer will be cleaned up. When this happens is up to the garbage collector, who will notice that the Dispose for the command has not been called, and invoke it. Of course, the GC may not run for minutes, hours or days depending on the situation. If the writer is holding an exclusive lock on say, a file, no other process will be able to open it, even though you're long finished.

The using block ensures the Dispose call is always made, and hence that the Close is always called, regardless of what control flow occurs.

10
  • How does it clean up? Does it call its own Dispose method in a finalizer?
    – xyz
    Jun 19, 2009 at 17:01
  • Typically for member references to unmanaged resources, the same cleanup code will be called from both. I've said "Invoke it" above, but there's no certainty that the Dispose function be called. The behavior will likely be very similar in both cases though. Jun 19, 2009 at 17:11
  • The GC will never call an object's Dispose method. If the object has a finaliser then that should eventually be called by the GC, and it's possible that the finaliser might then manually call Dispose (whether it does or not is an implementation detail).
    – LukeH
    Jun 19, 2009 at 17:15
  • Yes. The GC runs and detects all objects that have no roots. Any with a finalizer method are not collected queued then the finalizer thread is started. The finalizer runs the dispose method on the queued objects. (This is why you should follow the proper dispose pattern on your own objects - to make sure you do the correct thing when called from a finalizer or from a using block). This is also why it is import you use using because objects with a finalizer will get promoted to the next GC generation while the finalizer is run, so the survive a lot longer if not disposed of properly. Jun 19, 2009 at 17:21
  • @Adam Thanks for the explanation. I posted a (probably awkwardly worded) question about this area yesterday. A lot of people seem to fixate on re-re-explaining how using translates to try/catch/finally, which isn't helpful :) @Luke I didn't mean to imply that it will automatically be called. Just that the programmer would manually add the dispose/cleanup call to a finalizer.
    – xyz
    Jun 19, 2009 at 17:22
3

Wrapping the StreamWriter in a using block is pretty much equivalent of the following code:

StreamWriter writer;
try
{
    writer = new StreamWriter(Response.OutputStream);
    writer.WriteLine("col1,col2,col3");
    writer.WriteLine("1,2,3");
}
catch
{
    throw;
}
finally
{
    if (writer != null)
    {
        writer.Close();    
    }
}

While you could very well write this code yourself, it is so much easier to just put it in a using block.

2
  • 1
    closing the StreamWriter will clode the Respose.OutputStream as well. Just a warning if you find you data not making it to the client. Jun 19, 2009 at 17:27
  • 1
    Just omit the catch() {} part, it serves no purpose and can only be confusing.
    – ℍ ℍ
    Jun 19, 2009 at 17:40
1

In my opinion it's necessary to wrap any class that implements IDisposable in a using block. The fact that a class implements IDisposable means that the class has resources that need to be cleaned up.

1
  • 1
    What if you need to maintain that open state for a longer period of time. Such as Async sockets and streams. There is a reason you can implment IDisposible on your own. Also IDisposable will close the stream as well. Meaning that if you redirect content such as from a streamwriter as above and dispose of the streamwriter the streamwriter will close the stream that was passed to this. this may prevent the data from being sent to the client. Jun 19, 2009 at 17:23
1

My rule of thumb is, if I see Dispose listed in intellisense, I wrap it in a using block.

1
  • Good rule, but not enough. Some classes use an explicit implementation of IDisposable, effectively hiding Dispose(). Bad idea, but it is being used. You still need to apply using(){}
    – ℍ ℍ
    Jun 19, 2009 at 17:54
0

the using block calls dispose() when it ends. It's just a handy way of ensuring that resources are cleaned up in a timely manner.

2
  • Common misconception, Dispose is a way to tell consumers of your class that the object should be cleaned up, Finalizers are the way that the garbage collector cleans up when the object leaves scope or the program ends.
    – Guvante
    Jun 19, 2009 at 17:45
  • msdn.microsoft.com/en-us/library/… "Use this method to close or release unmanaged resources such as files, streams, and handles held by an instance of the class that implements this interface. This method is, by convention, used for all tasks associated with freeing resources held by an object, or preparing an object for reuse."
    – Robert
    Jun 20, 2009 at 3:10
0

In almost every case, if a class implements IDisposable, and if you're creating an instance of that class, then you need the using block.

5
  • @John -- are there any exceptions?
    – Jamie Ide
    Jun 19, 2009 at 17:01
  • Only WCF client proxies, as far as I know. This is due to a design error on the part of Microsoft. Jun 19, 2009 at 17:12
  • @John: how do you figure this is an error? There are times you need to leave the outgoing steam open and only close it when the client is complete. If you close it before the client downloaded all of the data the connection would be aborted and the client would never receive the data Jun 19, 2009 at 17:25
  • @Matthew: the design error is that it breaks when you use a using block on a WCF proxy. If an exception is unhandled in the block, Dispose is called, which calls Abort, which can throw its own, separate exception. I lose all information on the original problem. Jun 19, 2009 at 17:35
  • I actually consider StreamReader and StreamWriter to be exceptions to this rule. See my answer.
    – Joe White
    Jun 19, 2009 at 18:19
0

While it's good practice to always dipose disposable classes such as StreamWriter, as others point out, in this case it doesn't matter.

Response.OutputStream will be disposed by the ASP.NET infrastructure when it's finished processing your request.

StreamWriter assumes it "owns" a Stream passed to the constructor, and will therefore Close the stream when it's disposed. But in the sample you provide, the stream was instantiated outside your code, so there will be another owner who is responsible for the clean-up.

4
  • What about the StreamWriter? What if it held on to some unmanaged resources aside from the stream that it wraps? Jun 19, 2009 at 17:47
  • It doesn't have any other unmanaged resource. But I agree that it's good practice to dispose it, I'm just pointing out that in this situation, the stream will be disposed either way.
    – Joe
    Jun 19, 2009 at 18:07
  • If you don't dispose the StreamWriter, you do need to call Flush() when you're done. Otherwise part of your output may still be buffered in memory and not yet written to the stream.
    – Joe White
    Jun 19, 2009 at 18:18
  • @Joe White: "If you don't dispose the StreamWriter, you do need to call Flush()" - in the general case, yes. All I'm saying is that in this specific case, disposing (and therefore flushing) is handled by the ASP.NET infrastructure.
    – Joe
    Jun 19, 2009 at 20:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.