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.)
This commit is contained in:
Jon Skeet 2016-02-29 11:51:56 +00:00
parent 60a0d41a29
commit c0cf71bec9
5 changed files with 153 additions and 13 deletions

1
csharp/.gitignore vendored
View File

@ -33,3 +33,4 @@ mono/.libs
mono/*.exe
mono/*.dll
lib/protoc.exe
*.ncrunch*

View File

@ -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
}
}
}

View File

@ -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
}
}
}

View File

@ -51,8 +51,14 @@ 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>
@ -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,13 +142,27 @@ 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.
@ -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,

View File

@ -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,24 @@ 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 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 @@ 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. 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>
@ -659,6 +688,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>
@ -705,4 +758,4 @@ namespace Google.Protobuf
}
}
}
}
}