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.)
diff --git a/csharp/.gitignore b/csharp/.gitignore
index 07ea90a..c88f741 100644
--- a/csharp/.gitignore
+++ b/csharp/.gitignore
@@ -33,3 +33,4 @@
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 0e7cf04..239d3c9 100644
--- a/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs
+++ b/csharp/src/Google.Protobuf.Test/CodedInputStreamTest.cs
@@ -572,5 +572,27 @@
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 3297fe8..48bf6d6 100644
--- a/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs
+++ b/csharp/src/Google.Protobuf.Test/CodedOutputStreamTest.cs
@@ -387,5 +387,33 @@
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 1c02d95..ce6856d 100644
--- a/csharp/src/Google.Protobuf/CodedInputStream.cs
+++ b/csharp/src/Google.Protobuf/CodedInputStream.cs
@@ -51,9 +51,15 @@
/// 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 @@
}
/// <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 @@
}
/// <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 @@
/// </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 d6355f0..6211aac 100644
--- a/csharp/src/Google.Protobuf/CodedOutputStream.cs
+++ b/csharp/src/Google.Protobuf/CodedOutputStream.cs
@@ -55,7 +55,7 @@
/// and <c>MapField<TKey, TValue></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 @@
/// </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,24 @@
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 CodedOutputStream which write to the given stream.
+ /// 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>
- public CodedOutputStream(Stream output) : this(output, DefaultBufferSize)
+ /// <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)
{
}
@@ -112,9 +117,33 @@
/// 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. 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>
+ /// <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)
+ {
+ }
+
+ /// <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.</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 @@
}
/// <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 @@
}
}
}
-}
\ No newline at end of file
+}
