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

* Subclasses of `Stream` now have `Close` call `base.Close` to ensure disposal.
* Add DeleteOnClose to File opening.
* Remove explicit delete of file.
* Remove explicit close of substream.
* Since no longer deleting explicitly, no longer need `_overflowPath` member.

Author: TomFinley

src/Microsoft.ML.Core/Utilities/HybridMemoryStream.cs

@@ -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
@@ -123,27 +121,24 @@ protected override void Dispose(bool disposing)
                     var overflow = _overflowStream;
                     _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();
         }
 
@@ -164,9 +159,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

src/Microsoft.ML.Core/Utilities/TextReaderStream.cs

@@ -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;
@@ -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)
@@ -96,6 +88,7 @@ public override void Close()
         protected override void Dispose(bool disposing)
         {
             _baseReader.Dispose();
+            base.Dispose(disposing);
         }
 
         public override void Flush()
@@ -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.");
     }
 }