From 7a6dd041b9dee6c0a9cfcf2addb3d090be460444 Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Sun, 8 Nov 2015 21:53:01 +0000 Subject: [PATCH 1/2] Dispose of MemoryPool2; suppress its finalizers --- .../Http/ListenerContext.cs | 5 +-- .../Infrastructure/KestrelThread.cs | 7 +++ .../Infrastructure/MemoryPool2.cs | 45 ++++++++----------- .../Infrastructure/MemoryPoolBlock2.cs | 5 +++ .../FrameTests.cs | 27 ++++++----- .../TestInput.cs | 13 ++++-- 6 files changed, 56 insertions(+), 46 deletions(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/Http/ListenerContext.cs b/src/Microsoft.AspNet.Server.Kestrel/Http/ListenerContext.cs index 581720041..0ae440415 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Http/ListenerContext.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Http/ListenerContext.cs @@ -10,13 +10,11 @@ public class ListenerContext : ServiceContext { public ListenerContext() { - Memory2 = new MemoryPool2(); } public ListenerContext(ServiceContext serviceContext) : base(serviceContext) { - Memory2 = new MemoryPool2(); } public ListenerContext(ListenerContext listenerContext) @@ -25,7 +23,6 @@ public ListenerContext(ListenerContext listenerContext) ServerAddress = listenerContext.ServerAddress; Thread = listenerContext.Thread; Application = listenerContext.Application; - Memory2 = listenerContext.Memory2; Log = listenerContext.Log; } @@ -35,6 +32,6 @@ public ListenerContext(ListenerContext listenerContext) public RequestDelegate Application { get; set; } - public MemoryPool2 Memory2 { get; set; } + public MemoryPool2 Memory2 => Thread.Memory2; } } diff --git a/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/KestrelThread.cs b/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/KestrelThread.cs index a8608de70..50b404420 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/KestrelThread.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/KestrelThread.cs @@ -37,6 +37,7 @@ public class KestrelThread public KestrelThread(KestrelEngine engine) { _engine = engine; + Memory2 = new MemoryPool2(); _appLifetime = engine.AppLifetime; _log = engine.Log; _loop = new UvLoopHandle(_log); @@ -46,6 +47,9 @@ public KestrelThread(KestrelEngine engine) } public UvLoopHandle Loop { get { return _loop; } } + + public MemoryPool2 Memory2 { get; } + public ExceptionDispatchInfo FatalError { get { return _closeError; } } public Action, IntPtr> QueueCloseHandle { get; internal set; } @@ -61,6 +65,7 @@ public void Stop(TimeSpan timeout) { if (!_initCompleted) { + Memory2.Dispose(); return; } @@ -94,6 +99,8 @@ public void Stop(TimeSpan timeout) } } + Memory2.Dispose(); + if (_closeError != null) { _closeError.Throw(); diff --git a/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPool2.cs b/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPool2.cs index 86561516c..60d39727b 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPool2.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPool2.cs @@ -48,11 +48,6 @@ public class MemoryPool2 : IDisposable /// private readonly ConcurrentStack _slabs = new ConcurrentStack(); - /// - /// This is part of implementing the IDisposable pattern. - /// - private bool _disposedValue = false; // To detect redundant calls - /// /// Called to take a block from the pool. /// @@ -137,39 +132,35 @@ public void Return(MemoryPoolBlock2 block) protected virtual void Dispose(bool disposing) { - if (!_disposedValue) + MemoryPoolSlab2 slab; + while (_slabs.TryPop(out slab)) { - if (disposing) - { - MemoryPoolSlab2 slab; - while (_slabs.TryPop(out slab)) - { - // dispose managed state (managed objects). - slab.Dispose(); - } - } - - // N/A: free unmanaged resources (unmanaged objects) and override a finalizer below. - - // N/A: set large fields to null. + // Free pinned objects + slab.Dispose(); + } - _disposedValue = true; + MemoryPoolBlock2 block; + while (_blocks.TryPop(out block)) + { + // Deactivate finalizers + block.Dispose(); } } - // N/A: override a finalizer only if Dispose(bool disposing) above has code to free unmanaged resources. - // ~MemoryPool2() { - // // Do not change this code. Put cleanup code in Dispose(bool disposing) above. - // Dispose(false); - // } + // Disposing slabs unpin memory so finalizer is needed. + ~MemoryPool2() + { + // Do not change this code. Put cleanup code in Dispose(bool disposing) above. + Dispose(false); + } // This code added to correctly implement the disposable pattern. public void Dispose() { // Do not change this code. Put cleanup code in Dispose(bool disposing) above. Dispose(true); - // N/A: uncomment the following line if the finalizer is overridden above. - // GC.SuppressFinalize(this); + + GC.SuppressFinalize(this); } } } diff --git a/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPoolBlock2.cs b/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPoolBlock2.cs index a93e03186..5ae74457a 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPoolBlock2.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPoolBlock2.cs @@ -173,5 +173,10 @@ public MemoryPoolIterator2 GetIterator() { return new MemoryPoolIterator2(this); } + + internal void Dispose() + { + GC.SuppressFinalize(this); + } } } diff --git a/test/Microsoft.AspNet.Server.KestrelTests/FrameTests.cs b/test/Microsoft.AspNet.Server.KestrelTests/FrameTests.cs index 8266a1c2e..1d3889001 100644 --- a/test/Microsoft.AspNet.Server.KestrelTests/FrameTests.cs +++ b/test/Microsoft.AspNet.Server.KestrelTests/FrameTests.cs @@ -48,22 +48,25 @@ public void ChunkedPrefixMustBeHexCrLfWithoutLeadingZeros(int dataCount, string [InlineData("Connection:\r\n \r\nCookie \r\n", 1)] public void EmptyHeaderValuesCanBeParsed(string rawHeaders, int numHeaders) { - var socketInput = new SocketInput(new MemoryPool2()); - var headerCollection = new FrameRequestHeaders(); + using (var memory = new MemoryPool2()) + { + var socketInput = new SocketInput(memory); + var headerCollection = new FrameRequestHeaders(); - var headerArray = Encoding.ASCII.GetBytes(rawHeaders); - var inputBuffer = socketInput.IncomingStart(headerArray.Length); - Buffer.BlockCopy(headerArray, 0, inputBuffer.Data.Array, inputBuffer.Data.Offset, headerArray.Length); - socketInput.IncomingComplete(headerArray.Length, null); + var headerArray = Encoding.ASCII.GetBytes(rawHeaders); + var inputBuffer = socketInput.IncomingStart(headerArray.Length); + Buffer.BlockCopy(headerArray, 0, inputBuffer.Data.Array, inputBuffer.Data.Offset, headerArray.Length); + socketInput.IncomingComplete(headerArray.Length, null); - var success = Frame.TakeMessageHeaders(socketInput, headerCollection); + var success = Frame.TakeMessageHeaders(socketInput, headerCollection); - Assert.True(success); - Assert.Equal(numHeaders, headerCollection.Count()); + Assert.True(success); + Assert.Equal(numHeaders, headerCollection.Count()); - // Assert TakeMessageHeaders consumed all the input - var scan = socketInput.ConsumingStart(); - Assert.True(scan.IsEnd); + // Assert TakeMessageHeaders consumed all the input + var scan = socketInput.ConsumingStart(); + Assert.True(scan.IsEnd); + } } } } diff --git a/test/Microsoft.AspNet.Server.KestrelTests/TestInput.cs b/test/Microsoft.AspNet.Server.KestrelTests/TestInput.cs index 3db112560..390d09f51 100644 --- a/test/Microsoft.AspNet.Server.KestrelTests/TestInput.cs +++ b/test/Microsoft.AspNet.Server.KestrelTests/TestInput.cs @@ -9,21 +9,28 @@ namespace Microsoft.AspNet.Server.KestrelTests { - class TestInput : IConnectionControl, IFrameControl + class TestInput : IConnectionControl, IFrameControl, IDisposable { + private readonly MemoryPool2 _memory2; + public TestInput() { var memory = new MemoryPool(); - var memory2 = new MemoryPool2(); + _memory2 = new MemoryPool2(); FrameContext = new FrameContext { - SocketInput = new SocketInput(memory2), + SocketInput = new SocketInput(_memory2), Memory = memory, ConnectionControl = this, FrameControl = this }; } + public void Dispose() + { + _memory2.Dispose(); + } + public FrameContext FrameContext { get; set; } public void Add(string text, bool fin = false) From ee4d9c35fb96c98c51d7d1256ef52f016753d2db Mon Sep 17 00:00:00 2001 From: Ben Adams Date: Fri, 13 Nov 2015 09:21:57 +0000 Subject: [PATCH 2/2] Dispose in tests --- .../MessageBodyTests.cs | 82 ++++++++++--------- 1 file changed, 44 insertions(+), 38 deletions(-) diff --git a/test/Microsoft.AspNet.Server.KestrelTests/MessageBodyTests.cs b/test/Microsoft.AspNet.Server.KestrelTests/MessageBodyTests.cs index b5dbfc021..377fe7486 100644 --- a/test/Microsoft.AspNet.Server.KestrelTests/MessageBodyTests.cs +++ b/test/Microsoft.AspNet.Server.KestrelTests/MessageBodyTests.cs @@ -19,66 +19,72 @@ public class MessageBodyTests [Fact] public void Http10ConnectionClose() { - var input = new TestInput(); - var body = MessageBody.For("HTTP/1.0", new Dictionary(), input.FrameContext); - var stream = new FrameRequestStream(body); + using (var input = new TestInput()) + { + var body = MessageBody.For("HTTP/1.0", new Dictionary(), input.FrameContext); + var stream = new FrameRequestStream(body); - input.Add("Hello", true); + input.Add("Hello", true); - var buffer1 = new byte[1024]; - var count1 = stream.Read(buffer1, 0, 1024); - AssertASCII("Hello", new ArraySegment(buffer1, 0, 5)); + var buffer1 = new byte[1024]; + var count1 = stream.Read(buffer1, 0, 1024); + AssertASCII("Hello", new ArraySegment(buffer1, 0, 5)); - var buffer2 = new byte[1024]; - var count2 = stream.Read(buffer2, 0, 1024); - Assert.Equal(0, count2); + var buffer2 = new byte[1024]; + var count2 = stream.Read(buffer2, 0, 1024); + Assert.Equal(0, count2); + } } [Fact] public async Task Http10ConnectionCloseAsync() { - var input = new TestInput(); - var body = MessageBody.For("HTTP/1.0", new Dictionary(), input.FrameContext); - var stream = new FrameRequestStream(body); + using (var input = new TestInput()) + { + var body = MessageBody.For("HTTP/1.0", new Dictionary(), input.FrameContext); + var stream = new FrameRequestStream(body); - input.Add("Hello", true); + input.Add("Hello", true); - var buffer1 = new byte[1024]; - var count1 = await stream.ReadAsync(buffer1, 0, 1024); - AssertASCII("Hello", new ArraySegment(buffer1, 0, 5)); + var buffer1 = new byte[1024]; + var count1 = await stream.ReadAsync(buffer1, 0, 1024); + AssertASCII("Hello", new ArraySegment(buffer1, 0, 5)); - var buffer2 = new byte[1024]; - var count2 = await stream.ReadAsync(buffer2, 0, 1024); - Assert.Equal(0, count2); + var buffer2 = new byte[1024]; + var count2 = await stream.ReadAsync(buffer2, 0, 1024); + Assert.Equal(0, count2); + } } [Fact] public async Task CanHandleLargeBlocks() { - var input = new TestInput(); - var body = MessageBody.For("HTTP/1.0", new Dictionary(), input.FrameContext); - var stream = new FrameRequestStream(body); + using (var input = new TestInput()) + { + var body = MessageBody.For("HTTP/1.0", new Dictionary(), input.FrameContext); + var stream = new FrameRequestStream(body); - // Input needs to be greater than 4032 bytes to allocate a block not backed by a slab. - var largeInput = new string('a', 8192); + // Input needs to be greater than 4032 bytes to allocate a block not backed by a slab. + var largeInput = new string('a', 8192); - input.Add(largeInput, true); - // Add a smaller block to the end so that SocketInput attempts to return the large - // block to the memory pool. - input.Add("Hello", true); + input.Add(largeInput, true); + // Add a smaller block to the end so that SocketInput attempts to return the large + // block to the memory pool. + input.Add("Hello", true); - var readBuffer = new byte[8192]; + var readBuffer = new byte[8192]; - var count1 = await stream.ReadAsync(readBuffer, 0, 8192); - Assert.Equal(8192, count1); - AssertASCII(largeInput, new ArraySegment(readBuffer, 0, 8192)); + var count1 = await stream.ReadAsync(readBuffer, 0, 8192); + Assert.Equal(8192, count1); + AssertASCII(largeInput, new ArraySegment(readBuffer, 0, 8192)); - var count2 = await stream.ReadAsync(readBuffer, 0, 8192); - Assert.Equal(5, count2); - AssertASCII("Hello", new ArraySegment(readBuffer, 0, 5)); + var count2 = await stream.ReadAsync(readBuffer, 0, 8192); + Assert.Equal(5, count2); + AssertASCII("Hello", new ArraySegment(readBuffer, 0, 5)); - var count3 = await stream.ReadAsync(readBuffer, 0, 8192); - Assert.Equal(0, count3); + var count3 = await stream.ReadAsync(readBuffer, 0, 8192); + Assert.Equal(0, count3); + } } private void AssertASCII(string expected, ArraySegment actual)