diff --git a/UnitTests/Tests.cs b/UnitTests/Tests.cs index 037a24fa..00d02a9c 100644 --- a/UnitTests/Tests.cs +++ b/UnitTests/Tests.cs @@ -1427,8 +1427,8 @@ public void DisposeReturnsLargeBuffer() Assert.That(memMgr.LargePoolInUseSize, Is.EqualTo(0)); } - [Test, ExpectedException(typeof(InvalidOperationException))] - public void DisposeTwiceThrowsException() + [Test] + public void DisposeTwiceDoesNotThrowException() { var stream = this.GetDefaultStream(); stream.Dispose(); diff --git a/src/RecyclableMemoryStream.cs b/src/RecyclableMemoryStream.cs index 81f5872a..7eb439b2 100644 --- a/src/RecyclableMemoryStream.cs +++ b/src/RecyclableMemoryStream.cs @@ -40,7 +40,7 @@ namespace Microsoft.IO /// leads to continual memory growth as each stream approaches the maximum allowed size. /// 3. Memory copying - Each time a MemoryStream grows, all the bytes are copied into new buffers. /// This implementation only copies the bytes when GetBuffer is called. - /// 4. Memory fragmentation - By using homogenous buffer sizes, it ensures that blocks of memory + /// 4. Memory fragmentation - By using homogeneous buffer sizes, it ensures that blocks of memory /// can be easily reused. /// /// The stream is implemented on top of a series of uniformly-sized blocks. As the stream's length grows, @@ -140,9 +140,8 @@ internal RecyclableMemoryStreamManager MemoryManager /// /// The memory manager public RecyclableMemoryStream(RecyclableMemoryStreamManager memoryManager) - : this(memoryManager, null) + : this(memoryManager, null, 0, null) { - } /// @@ -151,9 +150,8 @@ public RecyclableMemoryStream(RecyclableMemoryStreamManager memoryManager) /// The memory manager /// A string identifying this stream for logging and debugging purposes public RecyclableMemoryStream(RecyclableMemoryStreamManager memoryManager, string tag) - : this(memoryManager, tag, 0) + : this(memoryManager, tag, 0, null) { - } /// @@ -208,7 +206,6 @@ public RecyclableMemoryStream(RecyclableMemoryStreamManager memoryManager, strin #endregion #region Dispose and Finalize - ~RecyclableMemoryStream() { this.Dispose(false); @@ -231,7 +228,7 @@ protected override void Dispose(bool disposing) } Events.Write.MemoryStreamDoubleDispose(this.id, this.tag, this.allocationStack, this.disposeStack, doubleDisposeStack); - throw new InvalidOperationException("Cannot dispose of RecyclableMemoryStream twice"); + return; } Events.Write.MemoryStreamDisposed(this.id, this.tag); @@ -295,7 +292,6 @@ public override void Close() { this.Dispose(true); } - #endregion #region MemoryStream overrides @@ -325,7 +321,11 @@ public override int Capacity } return 0; } - set { this.EnsureCapacity(value); } + set + { + this.CheckDisposed(); + this.EnsureCapacity(value); + } } private int length; @@ -358,6 +358,7 @@ public override long Position } set { + this.CheckDisposed(); if (value < 0) { throw new ArgumentOutOfRangeException("value", "value must be non-negative"); @@ -368,7 +369,7 @@ public override long Position throw new ArgumentOutOfRangeException("value", "value cannot be more than " + MaxStreamLength); } - this.position= (int)value; + this.position = (int)value; } } @@ -430,7 +431,7 @@ public override byte[] GetBuffer() // it's possible that people will manipulate the buffer directly // and set the length afterward. Capacity sets the expectation // for the size of the buffer. - var newBuffer = this.MemoryManager.GetLargeBuffer(this.Capacity, this.tag); + var newBuffer = this.memoryManager.GetLargeBuffer(this.Capacity, this.tag); // InternalRead will check for existence of largeBuffer, so make sure we // don't set it until after we've copied the data. @@ -538,46 +539,42 @@ public override void Write(byte[] buffer, int offset, int count) throw new ArgumentException("count must be greater than buffer.Length - offset"); } + int blockSize = this.memoryManager.BlockSize; + long end = this.Position + count; // Check for overflow - if (this.Position + count > MaxStreamLength) + if (end > MaxStreamLength) { throw new IOException("Maximum capacity exceeded"); } - - int end = (int)this.Position + count; - int blockSize = this.memoryManager.BlockSize; - - int requiredBuffers = (end + blockSize - 1) / blockSize; + long requiredBuffers = (end + blockSize - 1) / blockSize; if (requiredBuffers * blockSize > MaxStreamLength) { throw new IOException("Maximum capacity exceeded"); } - EnsureCapacity(end); + this.EnsureCapacity((int)end); if (this.largeBuffer == null) { int bytesRemaining = count; int bytesWritten = 0; - int currentBlockIndex = this.OffsetToBlockIndex(this.position); - - int blockOffset = this.OffsetToBlockOffset(this.position); + var blockAndOffset = this.GetBlockAndRelativeOffset(this.position); while (bytesRemaining > 0) { - byte[] currentBlock = this.blocks[currentBlockIndex]; - int remainingInBlock = blockSize - blockOffset; + byte[] currentBlock = this.blocks[blockAndOffset.Block]; + int remainingInBlock = blockSize - blockAndOffset.Offset; int amountToWriteInBlock = Math.Min(remainingInBlock, bytesRemaining); - Buffer.BlockCopy(buffer, offset + bytesWritten, currentBlock, blockOffset, amountToWriteInBlock); + Buffer.BlockCopy(buffer, offset + bytesWritten, currentBlock, blockAndOffset.Offset, amountToWriteInBlock); bytesRemaining -= amountToWriteInBlock; bytesWritten += amountToWriteInBlock; - currentBlockIndex++; - blockOffset = 0; + ++blockAndOffset.Block; + blockAndOffset.Offset = 0; } } else @@ -603,7 +600,7 @@ public override string ToString() /// Object has been disposed public override void WriteByte(byte value) { - CheckDisposed(); + this.CheckDisposed(); this.byteBuffer[0] = value; this.Write(this.byteBuffer, 0, 1); } @@ -623,9 +620,8 @@ public override int ReadByte() byte value = 0; if (this.largeBuffer == null) { - int block = OffsetToBlockIndex(this.position); - int blockOffset = OffsetToBlockOffset(this.position); - value = this.blocks[block][blockOffset]; + var blockAndOffset = this.GetBlockAndRelativeOffset(this.position); + value = this.blocks[blockAndOffset.Block][blockAndOffset.Offset]; } else { @@ -705,6 +701,7 @@ public override long Seek(long offset, SeekOrigin loc) /// Important: This does a synchronous write, which may not be desired in some situations public override void WriteTo(Stream stream) { + this.CheckDisposed(); if (stream == null) { throw new ArgumentNullException("stream"); @@ -733,7 +730,6 @@ public override void WriteTo(Stream stream) #endregion #region Helper Methods - private void CheckDisposed() { if (this.disposed) @@ -750,21 +746,20 @@ private int InternalRead(byte[] buffer, int offset, int count, int fromPosition) } if (this.largeBuffer == null) { - int currentBlock = this.OffsetToBlockIndex(fromPosition); + var blockAndOffset = this.GetBlockAndRelativeOffset(fromPosition); int bytesWritten = 0; int bytesRemaining = Math.Min(count, this.length - fromPosition); - int blockOffset = this.OffsetToBlockOffset(fromPosition); while (bytesRemaining > 0) { - int amountToCopy = Math.Min(this.blocks[currentBlock].Length - blockOffset, bytesRemaining); - Buffer.BlockCopy(this.blocks[currentBlock], blockOffset, buffer, bytesWritten + offset, amountToCopy); + int amountToCopy = Math.Min(this.blocks[blockAndOffset.Block].Length - blockAndOffset.Offset, bytesRemaining); + Buffer.BlockCopy(this.blocks[blockAndOffset.Block], blockAndOffset.Offset, buffer, bytesWritten + offset, amountToCopy); bytesWritten += amountToCopy; bytesRemaining -= amountToCopy; - ++currentBlock; - blockOffset = 0; + ++blockAndOffset.Block; + blockAndOffset.Offset = 0; } return bytesWritten; } @@ -776,14 +771,22 @@ private int InternalRead(byte[] buffer, int offset, int count, int fromPosition) } } - private int OffsetToBlockIndex(int offset) + private struct BlockAndOffset { - return offset / this.memoryManager.BlockSize; + public int Block; + public int Offset; + + public BlockAndOffset(int block, int offset) + { + this.Block = block; + this.Offset = offset; + } } - private int OffsetToBlockOffset(int offset) + private BlockAndOffset GetBlockAndRelativeOffset(int offset) { - return offset % this.memoryManager.BlockSize; + var blockSize = this.memoryManager.BlockSize; + return new BlockAndOffset(offset / blockSize, offset % blockSize); } private void EnsureCapacity(int newCapacity) @@ -808,7 +811,7 @@ private void EnsureCapacity(int newCapacity) { while (this.Capacity < newCapacity) { - blocks.Add((this.MemoryManager.GetBlock())); + blocks.Add((this.memoryManager.GetBlock())); } } }