BinaryReader (et al) leave open

  • Thread starter William Stacey [MVP]
  • 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.
 
W

William Stacey [MVP]

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.
 
L

Lloyd Dupont

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.
}

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!!
 
D

Daniel O'Connell [C# MVP]

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.

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.
 
W

William Stacey [MVP]

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.
 
W

William Stacey [MVP]

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.
 
L

Lloyd Dupont

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.
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
 
W

William Stacey [MVP]

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?

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?

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

I disagree. What is your argument.
 
L

Lloyd Dupont

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;
}
 
W

William Stacey [MVP]

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.
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;
}
 
L

Lloyd Dupont

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.
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;
}
 
J

Jon Skeet [C# MVP]

William Stacey said:
Thanks Jon. I would think they could add this before 2.0 gets out the door.

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.

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

Maybe for 3.0...
 
W

William Stacey [MVP]

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.
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;
}
 
L

Lloyd Dupont

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.
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;
}
 
D

Damien

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.

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
 
J

Jon Skeet [C# MVP]

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

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.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top