Implement IDisposable for CodedInputStream and CodedOutputStream

This fixes issue #679 and issue #1282.
(The .gitignore change is just around ncrunch; I can put it in a separate PR if you really want.)

5 files changed, 152 insertions, 12 deletions

diff --git a/csharp/.gitignore b/csharp/.gitignore
index 07ea90ac..c88f741e 100644
--- a/csharp/.gitignore
+++ b/csharp/.gitignore
@@ -33,3 +33,4 @@ mono/.libs
 mono/*.exe
 mono/*.dll
 lib/protoc.exe
+*.ncrunch*
diff --git a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs
index 0e7cf04e..239d3c9a 100644
--- a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs
+++ b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs
@@ -572,5 +572,27 @@ namespace Google.Protobuf
             Assert.Throws<ArgumentOutOfRangeException>(() => CodedInputStream.CreateWithLimits(stream, 0, 1));

             Assert.Throws<ArgumentOutOfRangeException>(() => CodedInputStream.CreateWithLimits(stream, 1, 0));

         }

+

+        [Test]

+        public void Dispose_DisposesUnderlyingStream()

+        {

+            var memoryStream = new MemoryStream();

+            Assert.IsTrue(memoryStream.CanRead);

+            using (var cis = new CodedInputStream(memoryStream))

+            {

+            }

+            Assert.IsFalse(memoryStream.CanRead); // Disposed

+        }

+

+        [Test]

+        public void Dispose_WithLeaveOpen()

+        {

+            var memoryStream = new MemoryStream();

+            Assert.IsTrue(memoryStream.CanRead);

+            using (var cos = new CodedOutputStream(memoryStream, true))

+            {

+            }

+            Assert.IsTrue(memoryStream.CanRead); // We left the stream open

+        }

     }

 }
\ No newline at end of file
diff --git a/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs b/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs
index 3297fe87..48bf6d60 100644
--- a/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs
+++ b/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs
@@ -387,5 +387,33 @@ namespace Google.Protobuf
                 Assert.IsTrue(cin.IsAtEnd);

             }

         }

+

+        [Test]

+        public void Dispose_DisposesUnderlyingStream()

+        {

+            var memoryStream = new MemoryStream();

+            Assert.IsTrue(memoryStream.CanWrite);

+            using (var cos = new CodedOutputStream(memoryStream))

+            {

+                cos.WriteRawByte(0);

+                Assert.AreEqual(0, memoryStream.Position); // Not flushed yet

+            }

+            Assert.AreEqual(1, memoryStream.ToArray().Length); // Flushed data from CodedOutputStream to MemoryStream

+            Assert.IsFalse(memoryStream.CanWrite); // Disposed

+        }

+

+        [Test]

+        public void Dispose_WithLeaveOpen()

+        {

+            var memoryStream = new MemoryStream();

+            Assert.IsTrue(memoryStream.CanWrite);

+            using (var cos = new CodedOutputStream(memoryStream, true))

+            {

+                cos.WriteRawByte(0);

+                Assert.AreEqual(0, memoryStream.Position); // Not flushed yet

+            }

+            Assert.AreEqual(1, memoryStream.Position); // Flushed data from CodedOutputStream to MemoryStream

+            Assert.IsTrue(memoryStream.CanWrite); // We left the stream open

+        }

     }

 }
\ No newline at end of file
diff --git a/csharp/src/Google.Protobuf/CodedInputStream.cs b/csharp/src/Google.Protobuf/CodedInputStream.cs
index 1c02d951..ce6856d6 100644
--- a/csharp/src/Google.Protobuf/CodedInputStream.cs
+++ b/csharp/src/Google.Protobuf/CodedInputStream.cs
@@ -51,9 +51,15 @@ namespace Google.Protobuf
     /// and <see cref="MapField{TKey, TValue}"/> to serialize such fields.

     /// </para>

     /// </remarks>

-    public sealed class CodedInputStream

+    public sealed class CodedInputStream : IDisposable

     {

         /// <summary>

+        /// Whether to leave the underlying stream open when disposing of this stream.

+        /// This is always true when there's no stream.

+        /// </summary>

+        private readonly bool leaveOpen;

+

+        /// <summary>

         /// Buffer of data read from the stream or provided at construction time.

         /// </summary>

         private readonly byte[] buffer;

@@ -120,7 +126,7 @@ namespace Google.Protobuf
         }

 

         /// <summary>

-        /// Creates a new CodedInputStream that reads from the given byte array slice.

+        /// Creates a new <see cref="CodedInputStream"/> that reads from the given byte array slice.

         /// </summary>

         public CodedInputStream(byte[] buffer, int offset, int length)

             : this(null, ProtoPreconditions.CheckNotNull(buffer, "buffer"), offset, offset + length)

@@ -136,14 +142,28 @@ namespace Google.Protobuf
         }

 

         /// <summary>

-        /// Creates a new CodedInputStream reading data from the given stream.

+        /// Creates a new <see cref="CodedInputStream"/> reading data from the given stream, which will be disposed

+        /// when the returned object is disposed.

         /// </summary>

-        public CodedInputStream(Stream input) : this(input, new byte[BufferSize], 0, 0)

+        /// <param name="input">The stream to read from.</param>

+        public CodedInputStream(Stream input) : this(input, false)

         {

-            ProtoPreconditions.CheckNotNull(input, "input");

         }

 

         /// <summary>

+        /// Creates a new <see cref="CodedInputStream"/> reading data from the given stream.

+        /// </summary>

+        /// <param name="input">The stream to read from.</param>

+        /// <param name="leaveOpen"><c>true</c> to leave <paramref name="input"/> open when the returned

+        /// <c cref="CodedInputStream"/> is disposed; <c>false</c> to dispose of the given stream when the

+        /// returned object is disposed.</param>

+        public CodedInputStream(Stream input, bool leaveOpen)

+            : this(ProtoPreconditions.CheckNotNull(input, "input"), new byte[BufferSize], 0, 0)

+        {

+            this.leaveOpen = leaveOpen;

+        }

+        

+        /// <summary>

         /// Creates a new CodedInputStream reading data from the given

         /// stream and buffer, using the default limits.

         /// </summary>

@@ -246,6 +266,22 @@ namespace Google.Protobuf
         /// </value>

         public int RecursionLimit { get { return recursionLimit; } }

 

+        /// <summary>

+        /// Disposes of this instance, potentially closing any underlying stream.

+        /// </summary>

+        /// <remarks>

+        /// As there is no flushing to perform here, disposing of a <see cref="CodedInputStream"/> which

+        /// was constructed with the <c>leaveOpen</c> option parameter set to <c>true</c> (or one which

+        /// was constructed to read from a byte array) has no effect.

+        /// </remarks>

+        public void Dispose()

+        {

+            if (!leaveOpen)

+            {

+                input.Dispose();

+            }

+        }

+

         #region Validation

         /// <summary>

         /// Verifies that the last call to ReadTag() returned tag 0 - in other words,

diff --git a/csharp/src/Google.Protobuf/CodedOutputStream.cs b/csharp/src/Google.Protobuf/CodedOutputStream.cs
index d6355f03..6211aac3 100644
--- a/csharp/src/Google.Protobuf/CodedOutputStream.cs
+++ b/csharp/src/Google.Protobuf/CodedOutputStream.cs
@@ -55,7 +55,7 @@ namespace Google.Protobuf
     /// and <c>MapField&lt;TKey, TValue&gt;</c> to serialize such fields.

     /// </para>

     /// </remarks>

-    public sealed partial class CodedOutputStream

+    public sealed partial class CodedOutputStream : IDisposable

     {

         // "Local" copy of Encoding.UTF8, for efficiency. (Yes, it makes a difference.)

         internal static readonly Encoding Utf8Encoding = Encoding.UTF8;

@@ -65,6 +65,7 @@ namespace Google.Protobuf
         /// </summary>

         public static readonly int DefaultBufferSize = 4096;

 

+        private readonly bool leaveOpen;

         private readonly byte[] buffer;

         private readonly int limit;

         private int position;

@@ -91,20 +92,44 @@ namespace Google.Protobuf
             this.buffer = buffer;

             this.position = offset;

             this.limit = offset + length;

+            leaveOpen = true; // Simple way of avoiding trying to dispose of a null reference

         }

 

-        private CodedOutputStream(Stream output, byte[] buffer)

+        private CodedOutputStream(Stream output, byte[] buffer, bool leaveOpen)

         {

-            this.output = output;

+            this.output = ProtoPreconditions.CheckNotNull(output, nameof(output));

             this.buffer = buffer;

             this.position = 0;

             this.limit = buffer.Length;

+            this.leaveOpen = leaveOpen;

+        }

+

+        /// <summary>

+        /// Creates a new <see cref="CodedOutputStream" /> which write to the given stream, and disposes of that

+        /// stream when the returned <c>CodedOutputStream</c> is disposed.

+        /// </summary>

+        /// <param name="output">The stream to write to. It will be disposed when the returned <c>CodedOutputStream is disposed.</c></param>

+        public CodedOutputStream(Stream output) : this(output, DefaultBufferSize, false)

+        {

+        }

+

+        /// <summary>

+        /// Creates a new CodedOutputStream which write to the given stream and uses

+        /// the specified buffer size.

+        /// </summary>

+        /// <param name="output">The stream to write to. It will be disposed when the returned <c>CodedOutputStream is disposed.</c></param>

+        /// <param name="bufferSize">The size of buffer to use internally.</param>

+        public CodedOutputStream(Stream output, int bufferSize) : this(output, new byte[bufferSize], false)

+        {

         }

 

         /// <summary>

         /// Creates a new CodedOutputStream which write to the given stream.

         /// </summary>

-        public CodedOutputStream(Stream output) : this(output, DefaultBufferSize)

+        /// <param name="output">The stream to write to.</param>

+        /// <param name="leaveOpen">If <c>true</c>, <paramref name="output"/> is left open when the returned <c>CodedOutputStream</c> is disposed;

+        /// if <c>false</c>, the provided stream is disposed as well.</param>

+        public CodedOutputStream(Stream output, bool leaveOpen) : this(output, DefaultBufferSize, leaveOpen)

         {

         }

 

@@ -112,9 +137,13 @@ namespace Google.Protobuf
         /// Creates a new CodedOutputStream which write to the given stream and uses

         /// the specified buffer size.

         /// </summary>

-        public CodedOutputStream(Stream output, int bufferSize) : this(output, new byte[bufferSize])

+        /// <param name="output">The stream to write to.</param>

+        /// <param name="bufferSize">The size of buffer to use internally.</param>

+        /// <param name="leaveOpen">If <c>true</c>, <paramref name="output"/> is left open when the returned <c>CodedOutputStream</c> is disposed;

+        /// if <c>false</c>, the provided stream is disposed as well.</param>

+        public CodedOutputStream(Stream output, int bufferSize, bool leaveOpen) : this(output, new byte[bufferSize], leaveOpen)

         {

-        }    

+        }

         #endregion

 

         /// <summary>

@@ -660,6 +689,30 @@ namespace Google.Protobuf
         }

 

         /// <summary>

+        /// Flushes any buffered data and optionally closes the underlying stream, if any.

+        /// </summary>

+        /// <remarks>

+        /// <para>

+        /// By default, any underlying stream is closed by this method. To configure this behaviour,

+        /// use a constructor overload with a <c>leaveOpen</c> parameter. If this instance does not

+        /// have an underlying stream, this method does nothing.

+        /// </para>

+        /// <para>

+        /// For the sake of efficiency, calling this method does not prevent future write calls - but

+        /// if a later write ends up writing to a stream which has been disposed, that is likely to

+        /// fail. It is recommend that you not call any other methods after this.

+        /// </para>

+        /// </remarks>

+        public void Dispose()

+        {

+            Flush();

+            if (!leaveOpen)

+            {

+                output.Dispose();

+            }

+        }

+

+        /// <summary>

         /// Flushes any buffered data to the underlying stream (if there is one).

         /// </summary>

         public void Flush()

@@ -705,4 +758,4 @@ namespace Google.Protobuf
             }

         }

     }

-}
\ No newline at end of file
+}