From c0cf71bec95fabafb4860564aecd464ba59254ef Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Mon, 29 Feb 2016 11:51:56 +0000 Subject: 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.) --- csharp/.gitignore | 1 + .../Google.Protobuf.Test/CodedInputStreamTest.cs | 22 +++++++ .../Google.Protobuf.Test/CodedOutputStreamTest.cs | 28 +++++++++ csharp/src/Google.Protobuf/CodedInputStream.cs | 46 +++++++++++++-- csharp/src/Google.Protobuf/CodedOutputStream.cs | 67 +++++++++++++++++++--- 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(() => CodedInputStream.CreateWithLimits(stream, 0, 1)); Assert.Throws(() => 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,8 +51,14 @@ namespace Google.Protobuf /// and to serialize such fields. /// /// - public sealed class CodedInputStream + public sealed class CodedInputStream : IDisposable { + /// + /// Whether to leave the underlying stream open when disposing of this stream. + /// This is always true when there's no stream. + /// + private readonly bool leaveOpen; + /// /// Buffer of data read from the stream or provided at construction time. /// @@ -120,7 +126,7 @@ namespace Google.Protobuf } /// - /// Creates a new CodedInputStream that reads from the given byte array slice. + /// Creates a new that reads from the given byte array slice. /// public CodedInputStream(byte[] buffer, int offset, int length) : this(null, ProtoPreconditions.CheckNotNull(buffer, "buffer"), offset, offset + length) @@ -136,13 +142,27 @@ namespace Google.Protobuf } /// - /// Creates a new CodedInputStream reading data from the given stream. + /// Creates a new reading data from the given stream, which will be disposed + /// when the returned object is disposed. /// - public CodedInputStream(Stream input) : this(input, new byte[BufferSize], 0, 0) + /// The stream to read from. + public CodedInputStream(Stream input) : this(input, false) { - ProtoPreconditions.CheckNotNull(input, "input"); } + /// + /// Creates a new reading data from the given stream. + /// + /// The stream to read from. + /// true to leave open when the returned + /// is disposed; false to dispose of the given stream when the + /// returned object is disposed. + public CodedInputStream(Stream input, bool leaveOpen) + : this(ProtoPreconditions.CheckNotNull(input, "input"), new byte[BufferSize], 0, 0) + { + this.leaveOpen = leaveOpen; + } + /// /// Creates a new CodedInputStream reading data from the given /// stream and buffer, using the default limits. @@ -246,6 +266,22 @@ namespace Google.Protobuf /// public int RecursionLimit { get { return recursionLimit; } } + /// + /// Disposes of this instance, potentially closing any underlying stream. + /// + /// + /// As there is no flushing to perform here, disposing of a which + /// was constructed with the leaveOpen option parameter set to true (or one which + /// was constructed to read from a byte array) has no effect. + /// + public void Dispose() + { + if (!leaveOpen) + { + input.Dispose(); + } + } + #region Validation /// /// 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 MapField<TKey, TValue> to serialize such fields. /// /// - 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 /// 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; + } + + /// + /// Creates a new which write to the given stream, and disposes of that + /// stream when the returned CodedOutputStream is disposed. + /// + /// The stream to write to. It will be disposed when the returned CodedOutputStream is disposed. + public CodedOutputStream(Stream output) : this(output, DefaultBufferSize, false) + { + } + + /// + /// Creates a new CodedOutputStream which write to the given stream and uses + /// the specified buffer size. + /// + /// The stream to write to. It will be disposed when the returned CodedOutputStream is disposed. + /// The size of buffer to use internally. + public CodedOutputStream(Stream output, int bufferSize) : this(output, new byte[bufferSize], false) + { } /// /// Creates a new CodedOutputStream which write to the given stream. /// - public CodedOutputStream(Stream output) : this(output, DefaultBufferSize) + /// The stream to write to. + /// If true, is left open when the returned CodedOutputStream is disposed; + /// if false, the provided stream is disposed as well. + 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. /// - public CodedOutputStream(Stream output, int bufferSize) : this(output, new byte[bufferSize]) + /// The stream to write to. + /// The size of buffer to use internally. + /// If true, is left open when the returned CodedOutputStream is disposed; + /// if false, the provided stream is disposed as well. + public CodedOutputStream(Stream output, int bufferSize, bool leaveOpen) : this(output, new byte[bufferSize], leaveOpen) { - } + } #endregion /// @@ -659,6 +688,30 @@ namespace Google.Protobuf } } + /// + /// Flushes any buffered data and optionally closes the underlying stream, if any. + /// + /// + /// + /// By default, any underlying stream is closed by this method. To configure this behaviour, + /// use a constructor overload with a leaveOpen parameter. If this instance does not + /// have an underlying stream, this method does nothing. + /// + /// + /// 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. + /// + /// + public void Dispose() + { + Flush(); + if (!leaveOpen) + { + output.Dispose(); + } + } + /// /// Flushes any buffered data to the underlying stream (if there is one). /// @@ -705,4 +758,4 @@ namespace Google.Protobuf } } } -} \ No newline at end of file +} -- cgit v1.2.3