Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stream subclasses now have Close call base.Close to ensure disposal. #369

Merged
merged 4 commits into from
Jun 18, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 16 additions & 19 deletions src/Microsoft.ML.Core/Utilities/HybridMemoryStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,24 @@ public sealed class HybridMemoryStream : Stream
{
private MemoryStream _memStream;
private Stream _overflowStream;
private string _overflowPath;
private readonly int _overflowBoundary;
private const int _defaultMaxLen = 1 << 30;

private bool _disposed;

private Stream MyStream { get { return _memStream ?? _overflowStream; } }
private Stream MyStream => _memStream ?? _overflowStream;

private bool IsMemory { get { return _memStream != null; } }
private bool IsMemory => _memStream != null;

public override long Position
{
get { return MyStream.Position; }
set { Seek(value, SeekOrigin.Begin); }
public override long Position {
get => MyStream.Position;
set => Seek(value, SeekOrigin.Begin);
}

public override long Length { get { return MyStream.Length; } }
public override bool CanWrite { get { return MyStream.CanWrite; } }
public override bool CanSeek { get { return MyStream.CanSeek; } }
public override bool CanRead { get { return MyStream.CanRead; } }
public override long Length => MyStream.Length;
public override bool CanWrite => MyStream.CanWrite;
public override bool CanSeek => MyStream.CanSeek;
public override bool CanRead => MyStream.CanRead;

/// <summary>
/// Constructs an initially empty read-write stream. Once the number of
Expand Down Expand Up @@ -124,26 +122,25 @@ protected override void Dispose(bool disposing)
_overflowStream = null;
overflow.Dispose();
Contracts.AssertValue(_overflowPath);
File.Delete(_overflowPath);
_overflowPath = null;
}
_disposed = true;
AssertInvariants();
base.Dispose(disposing);
}
}

public override void Close()
{
AssertInvariants();
if (MyStream != null)
MyStream.Close();
// The base Stream class Close will call Dispose(bool).
base.Close();
}

public override void Flush()
{
AssertInvariants();
if (MyStream != null)
MyStream.Flush();
MyStream?.Flush();
AssertInvariants();
}

Expand All @@ -164,9 +161,9 @@ private void EnsureOverflow()
// been closed.
Contracts.Check(_memStream.CanRead, "attempt to perform operation on closed stream");

Contracts.Assert(_overflowPath == null);
_overflowPath = Path.GetTempFileName();
_overflowStream = new FileStream(_overflowPath, FileMode.Open, FileAccess.ReadWrite);
string overflowPath = Path.GetTempFileName();
_overflowStream = new FileStream(overflowPath, FileMode.Open, FileAccess.ReadWrite,
FileShare.None, bufferSize: 4096, FileOptions.DeleteOnClose);

// The documentation is not clear on this point, but the source code for
// memory stream makes clear that this buffer is exposable for a memory
Expand Down
27 changes: 7 additions & 20 deletions src/Microsoft.ML.Core/Utilities/TextReaderStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Microsoft.ML.Runtime.Internal.Utilities
/// compensates by inserting <c>\n</c> line feed characters at the end of every
/// input line, including the last one.
/// </summary>
public class TextReaderStream : Stream
public sealed class TextReaderStream : Stream
{
private readonly TextReader _baseReader;
private readonly Encoding _encoding;
Expand All @@ -38,19 +38,11 @@ public class TextReaderStream : Stream
public override bool CanWrite => false;

public override long Length
{
get
{
throw Contracts.ExceptNotSupp("Stream cannot determine length.");
}
}
=> throw Contracts.ExceptNotSupp("Stream cannot determine length.");

public override long Position
{
get
{
return _position;
}
get => _position;
set
{
if (value != Position)
Expand Down Expand Up @@ -96,6 +88,7 @@ public override void Close()
protected override void Dispose(bool disposing)
{
_baseReader.Dispose();
base.Dispose(disposing);
}

public override void Flush()
Expand Down Expand Up @@ -182,18 +175,12 @@ public override int ReadByte()
}

public override long Seek(long offset, SeekOrigin origin)
{
throw Contracts.ExceptNotSupp("Stream cannot seek.");
}
=> throw Contracts.ExceptNotSupp("Stream cannot seek.");

public override void Write(byte[] buffer, int offset, int count)
{
throw Contracts.ExceptNotSupp("Stream is not writable.");
}
=> throw Contracts.ExceptNotSupp("Stream is not writable.");

public override void SetLength(long value)
{
throw Contracts.ExceptNotSupp("Stream is not writable.");
}
=> throw Contracts.ExceptNotSupp("Stream is not writable.");
}
}