BinaryReader (et al) leave open

  • Thread starter Thread starter William Stacey [MVP]
  • Start date Start date
W

William Stacey [MVP]

Shouldn't BinaryReader and BinaryWriter (or any stream that takes another
stream) have an overload to LeaveInnerStream open.

bool leaveInnerStreamOpen = true;
using(BinaryReader br = new BinaryReader(innerStream, leaveInnerStreamOpen))
{
// Use the br stream, but don't close innerStream after Dispose.
}

That way you can pass stream around to helper methods that may wrap the
stream in another stream and not close the first stream after the helper is
done.
 
Thanks Jon. I would think they could add this before 2.0 gets out the door.
One more constructor overload and a property and Dispose has to change. Not
sure what that would break (If anything) however.
 
Shouldn't BinaryReader and BinaryWriter (or any stream that takes another
stream) have an overload to LeaveInnerStream open.

bool leaveInnerStreamOpen = true;
using(BinaryReader br = new BinaryReader(innerStream,
leaveInnerStreamOpen))
{
// Use the br stream, but don't close innerStream after Dispose.
}
Click to expand...

try this:
BinaryReader br = new BinaryReader(innerStream, leaveInnerStreamOpen);
// Use the br stream, but don't close innerStream after Dispose.

the reason you would call Dispose() is if you want to release the inner
resource, namely the stream.
don't use using, the reader would still be collected but you won't have your
stream closed!!
 
William Stacey said:
Thanks Jon. I would think they could add this before 2.0 gets out the
door. One more constructor overload and a property and Dispose has to
change. Not sure what that would break (If anything) however.
Click to expand...

It would make things a bit more difficult, one type has two different sets
of dispose semantics, so definitivly closing a file is a little harder.
 
The issue is BinaryReader does not currently have the "leaveInnerStreamOpen"
overload. So you can't code it that way. If you could, I would still use
the Using clause to close the reader. It would just not close the
innerstream itself.
 
Not sure I understand Daniel. A FileStream for example, does not take a
stream in the constructor, so I don't have worry about closing an
innerstream there. So that will not change. It is only when I "wrap" a FS
(for example) in something like a TextWriter or BinaryWriter. In many
cases, I don't want dispose on those streams to close the innerstream as I
want control over that and close it when done. TMK, this does not change
dispose semantics from the Users perspective as dispose still gets called as
normal.
 
The issue is BinaryReader does not currently have the
"leaveInnerStreamOpen" overload. So you can't code it that way. If you
could, I would still use the Using clause to close the reader. It would
just not close the innerstream itself.
Click to expand...
Ask your self: what means closing the BinaryReader?

A binary reader is nothing if itself if it doesn't have a Stream.
Close it means nothing if not closing the underlying stream.
I don't think they would ever had this constructor because: what's the point
of disposing it if not disposing of the iner stream?

I repeaat: Do't use using
 
Ask your self: what means closing the BinaryReader?
A binary reader is nothing if itself if it doesn't have a Stream.
Close it means nothing if not closing the underlying stream.
I don't think they would ever had this constructor because: what's the
point of disposing it if not disposing of the iner stream?
Click to expand...

BinaryReader is just a simple example. That said, the Dispose method in
BinaryReader does more then just close the innerstream. It also closes its
internal state fields. You could imagine other more complex streams that
*need to close down more stuff to close clean - regardless if it closes
innerstream or not. Check the SslStream class for examples of this.
I don't think they would ever had this constructor because: what's the
point of disposing it if not disposing of the inner stream?
Click to expand...

There may be other state you need to close as BinaryReader also sets a bunch
of flags. Look at the code.
I repeat: Do't use using
Click to expand...

I disagree. What is your argument.
 
I disagree. What is your argument.
I disagree with you, what is the use of Dipose() anyway?

Anyway as you are impervious to the voice of reson, I copy the code from BinaryReader.Dispose() below (coming straight out of reflector) and I would be keen to know why "it is important" to call Dispose() on it.
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
Stream stream1 = this.m_stream;
this.m_stream = null;
if (stream1 != null)
{
stream1.Close();
}
}
this.m_stream = null;
this.m_buffer = null;
this.m_decoder = null;
this.m_charBytes = null;
this.m_singleChar = null;
this.m_charBuffer = null;
}
 
For the same reason you want to dispose of any object. You want its internal state to reflect it is disposed/closed. You don't want any future code that has a ref to the object to be able to call methods on the disposed stream (i.e. Read()). So I want to dispose of your wrapper stream, but you may not want to dispose of the innerstream - however you may want to Flush() it in the Dispose method like:
protected override void Dispose(bool disposing)
{
try
{
if (!disposing)
{
return;
}
if (this.leaveStreamOpen)
{
this.innerStream.Flush();
}
else
{
this.innerStream.Close();
}
}
finally
{
base.Dispose(disposing);
}
}-- William Stacey [MVP] "Lloyd Dupont said:
I disagree. What is your argument.
Click to expand...
I disagree with you, what is the use of Dipose() anyway?

Anyway as you are impervious to the voice of reson, I copy the code from BinaryReader.Dispose() below (coming straight out of reflector) and I would be keen to know why "it is important" to call Dispose() on it.
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
Stream stream1 = this.m_stream;
this.m_stream = null;
if (stream1 != null)
{
stream1.Close();
}
}
this.m_stream = null;
this.m_buffer = null;
this.m_decoder = null;
this.m_charBytes = null;
this.m_singleChar = null;
this.m_charBuffer = null;
}
 
oh yeah, Flush(), my bad :-(
so what of

BinaryReader br = new BinraryReader(stream)
// do stuff
stream.Flush()

here you go, achieve the desired result.
doesn't it?
For the same reason you want to dispose of any object. You want its internal state to reflect it is disposed/closed. You don't want any future code that has a ref to the object to be able to call methods on the disposed stream (i.e. Read()). So I want to dispose of your wrapper stream, but you may not want to dispose of the innerstream - however you may want to Flush() it in the Dispose method like:
protected override void Dispose(bool disposing)
{
try
{
if (!disposing)
{
return;
}
if (this.leaveStreamOpen)
{
this.innerStream.Flush();
}
else
{
this.innerStream.Close();
}
}
finally
{
base.Dispose(disposing);
}
}-- William Stacey [MVP] "Lloyd Dupont said:
I disagree. What is your argument.
Click to expand...
I disagree with you, what is the use of Dipose() anyway?

Anyway as you are impervious to the voice of reson, I copy the code from BinaryReader.Dispose() below (coming straight out of reflector) and I would be keen to know why "it is important" to call Dispose() on it.
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
Stream stream1 = this.m_stream;
this.m_stream = null;
if (stream1 != null)
{
stream1.Close();
}
}
this.m_stream = null;
this.m_buffer = null;
this.m_decoder = null;
this.m_charBytes = null;
this.m_singleChar = null;
this.m_charBuffer = null;
}
 
William Stacey said:
Thanks Jon. I would think they could add this before 2.0 gets out the door.
Click to expand...

I don't think so, to be honest.
One more constructor overload and a property and Dispose has to change. Not
sure what that would break (If anything) however.
Click to expand...

Indeed - there'd be a fair amount of testing needed.

Maybe for 3.0...
 
No. Because when br goes out of scope and gets disposed it closes my stream. I don't want the stream disposed, hence the reason to topic started. What is needed is the leaveInnerStream switch. Remember innerstream could be a NetworkStream and I may have helper method that wrap that stream in a typical server program to operate on the NetworkStream. When these methods return they would close the NetworkStream unexpectedly.

--
William Stacey [MVP]

oh yeah, Flush(), my bad :-(
so what of

BinaryReader br = new BinraryReader(stream)
// do stuff
stream.Flush()

here you go, achieve the desired result.
doesn't it?
For the same reason you want to dispose of any object. You want its internal state to reflect it is disposed/closed. You don't want any future code that has a ref to the object to be able to call methods on the disposed stream (i.e. Read()). So I want to dispose of your wrapper stream, but you may not want to dispose of the innerstream - however you may want to Flush() it in the Dispose method like:
protected override void Dispose(bool disposing)
{
try
{
if (!disposing)
{
return;
}
if (this.leaveStreamOpen)
{
this.innerStream.Flush();
}
else
{
this.innerStream.Close();
}
}
finally
{
base.Dispose(disposing);
}
}-- William Stacey [MVP] "Lloyd Dupont said:
I disagree. What is your argument.
Click to expand...
I disagree with you, what is the use of Dipose() anyway?

Anyway as you are impervious to the voice of reson, I copy the code from BinaryReader.Dispose() below (coming straight out of reflector) and I would be keen to know why "it is important" to call Dispose() on it.
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
Stream stream1 = this.m_stream;
this.m_stream = null;
if (stream1 != null)
{
stream1.Close();
}
}
this.m_stream = null;
this.m_buffer = null;
this.m_decoder = null;
this.m_charBytes = null;
this.m_singleChar = null;
this.m_charBuffer = null;
}
 
I refer you to the source code again
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
Stream stream1 = this.m_stream;
this.m_stream = null;
if (stream1 != null)
{
stream1.Close();
}
}
this.m_stream = null;
this.m_buffer = null;
this.m_decoder = null;
this.m_charBytes = null;
this.m_singleChar = null;
this.m_charBuffer = null;
No. Because when br goes out of scope and gets disposed it closes my stream. I don't want the stream disposed, hence the reason to topic started. What is needed is the leaveInnerStream switch. Remember innerstream could be a NetworkStream and I may have helper method that wrap that stream in a typical server program to operate on the NetworkStream. When these methods return they would close the NetworkStream unexpectedly.

--
William Stacey [MVP]

oh yeah, Flush(), my bad :-(
so what of

BinaryReader br = new BinraryReader(stream)
// do stuff
stream.Flush()

here you go, achieve the desired result.
doesn't it?
For the same reason you want to dispose of any object. You want its internal state to reflect it is disposed/closed. You don't want any future code that has a ref to the object to be able to call methods on the disposed stream (i.e. Read()). So I want to dispose of your wrapper stream, but you may not want to dispose of the innerstream - however you may want to Flush() it in the Dispose method like:
protected override void Dispose(bool disposing)
{
try
{
if (!disposing)
{
return;
}
if (this.leaveStreamOpen)
{
this.innerStream.Flush();
}
else
{
this.innerStream.Close();
}
}
finally
{
base.Dispose(disposing);
}
}-- William Stacey [MVP] "Lloyd Dupont said:
I disagree. What is your argument.
Click to expand...
I disagree with you, what is the use of Dipose() anyway?

Anyway as you are impervious to the voice of reson, I copy the code from BinaryReader.Dispose() below (coming straight out of reflector) and I would be keen to know why "it is important" to call Dispose() on it.
protected virtual void Dispose(bool disposing)
{
if (disposing)
{
Stream stream1 = this.m_stream;
this.m_stream = null;
if (stream1 != null)
{
stream1.Close();
}
}
this.m_stream = null;
this.m_buffer = null;
this.m_decoder = null;
this.m_charBytes = null;
this.m_singleChar = null;
this.m_charBuffer = null;
}
 
William said:
Shouldn't BinaryReader and BinaryWriter (or any stream that takes another
stream) have an overload to LeaveInnerStream open.

bool leaveInnerStreamOpen = true;
using(BinaryReader br = new BinaryReader(innerStream, leaveInnerStreamOpen))
{
// Use the br stream, but don't close innerStream after Dispose.
}

That way you can pass stream around to helper methods that may wrap the
stream in another stream and not close the first stream after the helper is
done.
Click to expand...

Hi,

Wouldn't this impose on these wrapper-streams additional constraints -
such as the current seek position of the underlying stream? AFAIK,
there are currently no such guarantees - precisely before it closes the
stream when you finish with it, and it assumes once you pass it a
stream that it is in complete control.

That said, I believe all current implementations do leave the
underlying stream at the "expected" position at all times. So you could
probably interleave two readers on the same underlying stream (at least
until you Dispose one...)

Damien

Damien
 
William Stacey said:
No. Because when br goes out of scope and gets disposed it closes my stream.
Click to expand...

It only gets disposed if you have a using statement, however. I don't
believe BinaryReader has a finalizer, so that wouldn't dispose of it
either.
 
Back
Top