Calling close on StringReader (or not)

  • Thread starter Thread starter Brad Wood
  • Start date Start date
B

Brad Wood

I have a test routine where I want to call
new XmlTextReader( new StringReader( xmlString ) )
repeatedly. Structuring the code so that I call Close (which calls
Dispose) on the StringReader makes the code much fatter.

Is failing to call Close on each instance here really a bad thing? Is
it a matter of improper protocol?

When I run tests creating gobs of StringReaders and letting them go out
of scope w/o calling Close, there don't seem to be any resulting memory
leaks (just by looking at the windows task manager memory). I presume
this is because there really isn't any unmanaged resources referenced (I
can't seem to find any while looking at StringReader and parent
TextReader in Reflector).
 
1st: use using:
using(XmlTextReader xtr = new XmlTextReader( new StringReader( xmlString ) )
{
// blabla
}
reduce the bloatness much!

2: in the case of chained stream/reader the top level dispose is here only
to close lower level dispose.
So you just need to close the top level one, no need to close all
intermediaries.

3: closing a stream is "important" only if the lowest level stream is using
unmanaged resource such as a FileStream, or a SocketStream.

4: if you don't call dispose they would be reclaimed eventually anyway. But
closing them as soon as possible release system resource faster. In case of
FileStream, for exemple, the main interest is to unlock the file as soon as
it is no longer needed.
 
4: if you don't call dispose they would be reclaimed eventually anyway. But
closing them as soon as possible release system resource faster. In case of
FileStream, for exemple, the main interest is to unlock the file as soon as
it is no longer needed.
Click to expand...
There is something to note is that if you do call any of
Flush/Close/Dispose, the content is not flush at all even when the
stream goes out of scope and get collected by the GC. Of course it only
applied to writer, not reader. Personally, I always use "using" when
the object implements IDisposable. Even that it just wraps arround
StringReader instead of a unmanaged resource, there is nothing to
guarantee that fact, either in current version and future versions.

Thi
 
Truong Hong Thi said:
There is something to note is that if you do call any of
Flush/Close/Dispose, the content is not flush at all even when the
Click to expand...

Very true!
I always forgot about this one! Sigh...
 
Lloyd said:
1st: use using:
using(XmlTextReader xtr = new XmlTextReader( new StringReader( xmlString ) )
{
// blabla
}
reduce the bloatness much!
Click to expand...

This is not possible because the XmlTextReader doesn't implement
IDisposable. That is the crux of my problem in trying to chain these 2
objects.
3: closing a stream is "important" only if the lowest level stream is using
unmanaged resource such as a FileStream, or a SocketStream.
Click to expand...

This is the part I really don't understand. An object should Implement
IDisposable only when unmanaged resources are used. Neither the
StringReader or its parent seem to use any unmanaged resources (upon
brief check in Reflector), so I don't understand why it implements
IDisposable.
4: if you don't call dispose they would be reclaimed eventually anyway. But
closing them as soon as possible release system resource faster. In case of
FileStream, for exemple, the main interest is to unlock the file as soon as
it is no longer needed.
Click to expand...

This is as I suspected, but only if the object properly disposes of
those resources in it's finalizer, right? For the purposes of my test
routines I couldn't care less when things are released, but I suppose
that might be a dangerous habit to form.
 
Brad said:
This is not possible because the XmlTextReader doesn't implement
IDisposable. That is the crux of my problem in trying to chain these
2 objects.
Click to expand...

Note that this has been fixed in .NET 2.0.
This is the part I really don't understand. An object should
Implement IDisposable only when unmanaged resources are used.
Neither the StringReader or its parent seem to use any unmanaged
resources (upon brief check in Reflector), so I don't understand why
it implements IDisposable.
Click to expand...

It's a design trade-off. All TextReaders except StringReader wrap a
Stream object, which (ignoring MemoryStream) do require disposing.
Thus, these readers deal with unmanaged resources. It would be pretty
bad to be able to write

using (FileStream stream =
new FileStream(@"C:\Foo.txt", FileMode.Open))
{
}

but not

using (StreamReader reader = new StreamReader(
new FileStream(@"C:\Foo.txt", FileMode.Open)))
{
}

Note that the "Disposable" rule extends to every owner of an
IDisposable. It makes no difference whether you forget to close a
stream yourself or forget to tell a class that wraps a stream to close
it.

Also, making the base class TextReader not implement IDisposable but
delegate that to each derived class is terrible design choice, because
it prevents generic code like this:

using (TextReader reader = CreateReader()
{
}

CreateReader() can be returning any kind of reader, which can always
safely be disposed of. If there's one reader in the hierarchy that
violates this rule, there's no way of abstracting from implementation
details.
This is as I suspected, but only if the object properly disposes of
those resources in it's finalizer, right? For the purposes of my
test routines I couldn't care less when things are released, but I
suppose that might be a dangerous habit to form.
Click to expand...

Why be sloppier with your test code than your real code ;-)

Cheers,
 
Truong Hong Thi said:
There is something to note is that if you do call any of
Flush/Close/Dispose, the content is not flush at all even when the
stream goes out of scope and get collected by the GC. Of course it only
applied to writer, not reader. Personally, I always use "using" when
the object implements IDisposable. Even that it just wraps arround
StringReader instead of a unmanaged resource, there is nothing to
guarantee that fact, either in current version and future versions.
Click to expand...

That depends on the stream. I believe FileStream *does* flush when it
is finalized, for instance. (That can cause a problem in some
situations, in fact - if flushing throws an IOException, you never end
up properly disposing of the object.)
 
That depends on the stream. I believe FileStream *does* flush when it
is finalized, for instance.
Click to expand...
I have check the documentation about this.
- FileStread, SocketStream, CryptoStream do flush and dispose in
Finalize.
- MemoryStream, StringWriter do not. This is easy to understand since
nothing to flush/dispose. In fact, they do not implement Finalize.
- BufferedStream, StreamWriter do not flush/dispose in Finalize. In
fact, they do not implement Finalize at all, just inherit from object.
The StreamReader class does not implement Finalize either.

After thinking about this for some time, I figure out why they do not
implement Finalize. Each of these 3 wrap another stream. When things
go out of scope and eligible for being collected, the GC makes no
waranty whether the wrapper or the wrapped stream is collected first.
It is posible that when collecting the wrapper, the wrapped stream is
already be collected, and, as the result, it is too late to call its
Flush/Dispose or any other methods.

The rule of thumb is, as Floyd said, always call Dispose/Close on the
top level one.

Thi
 
Back
Top