views:

221

answers:

5

Am I right that you'd only need a using() for the outermost stream if you're doing e.g

MemoryStream mstr = new MemoryStream();

using(StreamWriter w = new StreamWriter(mstr)) {
    ....
}

As disposing the StreamWriter should also dispose/close the underlying stream, there's no need to do this ?:

using(MemoryStream mstr = new MemoryStream())
using(StreamWriter w = new StreamWriter(mstr)) {
    ....
}

(Note these are just examples, for how to dispose wrapped streams, not looking for alternatives like just use a StringWriter etc.)

+3  A: 

From looking at the StreamWriter.Dispose method in Reflector, it looks like the underlying streams get closed, but not disposed. I would put each stream in a "using" block in order to explicity dispose of them all.

David
actually the .Close on an underlying stream calls this.Dispose() therefore disposing itself.
Stan R.
That's what it does _today_, Stan. I don't think that's part of the contract.
John Saunders
@John: I was just saying that from "looking at Reflector" that Close does call Dispose. I am not saying to use it, I am just saying what Reflector actually shows.
Stan R.
@Stan R:my concerns is that some people reading this may not get that distinction, since an expert like yourself referred the question to Reflector.
John Saunders
@John: I never claimed to be an expert :)
Stan R.
@John: Stream.Close is documented as releasing any resources that the Stream is using: http://msdn.microsoft.com/en-us/library/system.io.stream.close.aspx This presumably includes other Streams.
R. Bemrose
Some other opinions:http://stackoverflow.com/questions/234059/is-a-memory-leak-created-if-a-memorystream-in-net-is-not-closed and mine + John's long winded conversation over it: http://stackoverflow.com/questions/384974/trying-to-store-xml-content-into-sql-server-2005-fails-encoding-problem/1091209#1091209 . I did previously take Stan, David + Bemrose's view about reflector, but changed to using the best practice being spread on SO. It's a shame that the advice isn't on MSDN, nor have I seen it in any C# books
Chris S
+4  A: 

My rule of thumb: If it implements IDisposable, dispose of it.

While currently (and probably forever), calling StreamWriter.Dispose() closes the underlying stream, other stream-derived classes you may use in the future may not. Also, it seems not to actually call Dispose() either, so non-MemoryStreams may not get properly disposed (although I can't think of any that would suffer from that right now).

So, while you can safely dispose only of the StreamWriter, I find it a much better practice to always use using blocks for disposables whenever possible.

Philip Rieck
Stream.Close() says "This method calls Dispose, specifying true to release all resources." In other words, calling Close() on a Stream automatically calls Dispose(true) on it.
R. Bemrose
@R Bemrose As I say though, the StreamWriter calls close (not dispose) on the underlying stream. The fact that the underlying stream calls dispose internally is an implementation detail, and I would consider relying on that implementation chain for all streams to be a bad practice, even if it does currently work. Personally, I'll dispose any thing that implements IDisposable and that's what I'm putting forward with my rule of thumb. again, it's not required.
Philip Rieck
Calling Dispose normally has the same effect as calling Close, plus other effects, such as suppressing finalize. So it's safe to use "using" instead of "Close".
Steven Sudit
A: 

It's really a better practice to just always use a using block.

"Always", except for the one well-known case of WCF proxy classes, where a design flaw can sometimes cause their Dispose methods to throw an exception, losing your original exception.

John Saunders
Never ran into that bug? Link to connect article?
Greg D
http://www.iserviceoriented.com/blog/post/Indisposable+-+WCF+Gotcha+1.aspx
John Saunders
+3  A: 

It's a good idea to put all dependent resources in their own using blocks from a readability and maintainability point of view as much as anything. e.g. in the below code, after the final brace, it's impossible to attempt to access mstr because it is scoped to within the block where it is valid:

using (MemoryStream mstr = new MemoryStream())
using (StreamWriter w = new StreamWriter(mstr) {
    ....
}

// cannot attempt to access mstr here

If you don't scope it like this, then it's still possible to access mstr outside the scope of where it is valid

MemoryStream mstr = new MemoryStream();
using (StreamWriter w = new StreamWriter(mstr) {
    ....
}

mstr.Write(...); // KABOOM! ObjectDisposedException occurs here!

So while it may not always be necessary (and in this case it isn't) it's a good idea as it both clarifies and enforces your intention as to its scope.

Greg Beech
this is really a better explanation.
Stan R.
+2  A: 

To answer your actual question. TextWriter's Dispose method

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

It calls a protected Dispose and passes true to it. From TextWriter.Dispose Method (Boolean)

When the disposing parameter is true, this method releases all resources held by any managed objects that this TextWriter references. This method invokes the Dispose method of each referenced object.

However it is best practice to wrap everything that implements IDisposable in a using block, because we cannot guarantee that that the protected Dipose method will always be called with the true parameter.

Stan R.