Merge pull request #38 from marfenij/prepare_for_pr

fix disposing issues and incorrect work in netcore2.0
dotarj · Mar 25, 2018 · 5a7f983 · 5a7f983
2 parents c469638 + dd6a168
commit 5a7f983
Show file tree

Hide file tree

Showing 12 changed files with 495 additions and 163 deletions.
diff --git a/src/protobuf-net-data/ProtoDataReader.cs b/src/protobuf-net-data/ProtoDataReader.cs
@@ -20,7 +20,9 @@ namespace ProtoBuf.Data
     using System.Data;
     using System.IO;
     using ProtoBuf.Data.Internal;
+#if NET45 || NETSTANDARD20
     using System.Runtime.CompilerServices;
+#endif
 
     /// <summary>
     /// A custom <see cref="System.Data.IDataReader"/> for de-serializing a protocol-buffer binary stream back
@@ -33,11 +35,11 @@ public sealed class ProtoDataReader : IDataReader
         private Stream stream;
         private object[] currentRow;
         private DataTable dataTable;
-        private bool disposed;
         private ProtoReader reader;
         private int currentField;
         private SubItemToken currentTableToken;
         private bool reachedEndOfCurrentTable;
+        private bool isClosed;
 
         /// <summary>
         /// Initializes a new instance of the <see cref="ProtoDataReader"/> class. 
@@ -92,7 +94,7 @@ public int Depth
             }
         }
 
-        public bool IsClosed { get; private set; }
+        public bool IsClosed => this.isClosed;
 
         /// <summary>
         /// Gets the number of rows changed, inserted, or deleted. 
@@ -341,8 +343,25 @@ public bool IsDBNull(int i)
 
         public void Close()
         {
-            stream.Close();
-            IsClosed = true;
+            if (this.reader != null)
+            {
+                this.reader.Dispose();
+                this.reader = null;
+            }
+
+            if (this.stream != null)
+            {
+                this.stream.Dispose();
+                this.stream = null;
+            }
+
+            if (this.dataTable != null)
+            {
+                this.dataTable.Dispose();
+                this.dataTable = null;
+            }
+
+            this.isClosed = true;
         }
 
         public bool NextResult()
@@ -399,36 +418,15 @@ public bool Read()
 
         public void Dispose()
         {
-            Dispose(true);
+            this.Dispose(true);
             GC.SuppressFinalize(this);
         }
 
         private void Dispose(bool disposing)
         {
-            if (!disposed)
+            if (disposing)
             {
-                if (disposing)
-                {
-                    if (reader != null)
-                    {
-                        reader.Dispose();
-                        reader = null;
-                    }
-
-                    if (stream != null)
-                    {
-                        stream.Dispose();
-                        stream = null;
-                    }
-
-                    if (dataTable != null)
-                    {
-                        dataTable.Dispose();
-                        dataTable = null;
-                    }
-                }
-
-                disposed = true;
+                this.Close();
             }
         }
 

diff --git a/src/protobuf-net-data/ProtoDataStream.cs b/src/protobuf-net-data/ProtoDataStream.cs
@@ -30,17 +30,19 @@ namespace ProtoBuf.Data
     /// <remarks>Not guaranteed to be thread safe.</remarks>
     public class ProtoDataStream : Stream
     {
+        private const int ProtoWriterBufferSize = 1024;
+
         /// <summary>
         /// Buffer size.
         /// </summary>
-        public const int DefaultBufferSize = 128 * 1024;
+        public const int DefaultBufferSize = 128 * ProtoWriterBufferSize;
 
         private readonly ProtoDataWriterOptions options;
         private readonly ProtoDataColumnFactory columnFactory;
 
         private IDataReader reader;
         private ProtoWriter writer;
-        private Stream bufferStream;
+        private CircularStream bufferStream;
         private bool disposed;
         private int resultIndex;
         private bool isHeaderWritten;
@@ -127,8 +129,8 @@ public ProtoDataStream(IDataReader reader, int bufferSize = DefaultBufferSize)
         /// You should not need to change this unless you have exceptionally
         /// large rows or an exceptionally high number of columns.</param>
         public ProtoDataStream(
-            IDataReader reader, 
-            ProtoDataWriterOptions options, 
+            IDataReader reader,
+            ProtoDataWriterOptions options,
             int bufferSize = DefaultBufferSize)
         {
             if (reader == null)
@@ -157,7 +159,7 @@ public ProtoDataStream(
 
         public override bool CanRead
         {
-            get { return true; }
+            get { return !disposed; }
         }
 
         public override bool CanSeek
@@ -179,6 +181,10 @@ public override long Position
         {
             get
             {
+                if (readerIsClosed)
+                {
+                    throw new InvalidOperationException("Reader is closed.");
+                }
                 return bufferStream.Position;
             }
 
@@ -222,33 +228,20 @@ public override void Write(byte[] buffer, int offset, int count)
             throw new InvalidOperationException("This is a stream for reading serialized bytes. Writing is not supported.");
         }
 
-        public override void Close()
-        {
-            readerIsClosed = true;
-            if (reader != null)
-            {
-                reader.Close();
-            }
-        }
-
         protected override void Dispose(bool disposing)
         {
             if (!disposed)
             {
                 if (disposing)
                 {
+                    CloseReader();
+
                     if (writer != null)
                     {
                         ((IDisposable)writer).Dispose();
                         writer = null;
                     }
 
-                    if (reader != null)
-                    {
-                        reader.Dispose();
-                        reader = null;
-                    }
-
                     if (bufferStream != null)
                     {
                         bufferStream.Dispose();
@@ -260,6 +253,17 @@ protected override void Dispose(bool disposing)
             }
         }
 
+        private void CloseReader()
+        {
+            if (this.reader != null)
+            {
+                this.reader.Dispose();
+                this.reader = null;
+            }
+
+            this.readerIsClosed = true;
+        }
+
         private void WriteHeaderIfRequired()
         {
             if (isHeaderWritten)
@@ -285,7 +289,9 @@ private void FillBuffer(int requestedLength)
             WriteHeaderIfRequired();
 
             // write the rows
-            while (bufferStream.Length < requestedLength)
+            while (this.bufferStream.Length < requestedLength &&
+                   // protobuf-net not always return 1024 byte, so buffer can owerflow
+                   bufferStream.Capacity - bufferStream.Length >= ProtoWriterBufferSize)
             {
                 // NB protobuf-net only flushes every 1024 bytes. So
                 // it might take a few iterations for bufferStream.Length to
@@ -308,8 +314,9 @@ private void FillBuffer(int requestedLength)
                     else
                     {
                         // All done, no more results.
+                        // little optimization
                         writer.Close();
-                        Close();
+                        CloseReader();
                     }
 
                     break;