Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Dispose of MemoryPool2; suppress its finalizers #343

Closed
wants to merge 2 commits into from
Closed

Dispose of MemoryPool2; suppress its finalizers #343

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a6dd04
Dispose of MemoryPool2; suppress its finalizers
benaadams Nov 8, 2015
ee4d9c3
Dispose in tests
benaadams Nov 13, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions src/Microsoft.AspNet.Server.Kestrel/Http/ListenerContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -25,7 +23,6 @@ public ListenerContext(ListenerContext listenerContext)
ServerAddress = listenerContext.ServerAddress;
Thread = listenerContext.Thread;
Application = listenerContext.Application;
Memory2 = listenerContext.Memory2;
Log = listenerContext.Log;
}

Expand All @@ -35,6 +32,6 @@ public ListenerContext(ListenerContext listenerContext)

public RequestDelegate Application { get; set; }

public MemoryPool2 Memory2 { get; set; }
public MemoryPool2 Memory2 => Thread.Memory2;
}
}
7 changes: 7 additions & 0 deletions src/Microsoft.AspNet.Server.Kestrel/Infrastructure/KestrelThread.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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<Action<IntPtr>, IntPtr> QueueCloseHandle { get; internal set; }
Expand All @@ -61,6 +65,7 @@ public void Stop(TimeSpan timeout)
{
if (!_initCompleted)
{
Memory2.Dispose();
return;
}

Expand Down Expand Up @@ -94,6 +99,8 @@ public void Stop(TimeSpan timeout)
}
}

Memory2.Dispose();

if (_closeError != null)
{
_closeError.Throw();
Expand Down
45 changes: 18 additions & 27 deletions src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPool2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,6 @@ public class MemoryPool2 : IDisposable
/// </summary>
private readonly ConcurrentStack<MemoryPoolSlab2> _slabs = new ConcurrentStack<MemoryPoolSlab2>();

/// <summary>
/// This is part of implementing the IDisposable pattern.
/// </summary>
private bool _disposedValue = false; // To detect redundant calls

/// <summary>
/// Called to take a block from the pool.
/// </summary>
Expand Down Expand Up @@ -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);
}
}
}
5 changes: 5 additions & 0 deletions src/Microsoft.AspNet.Server.Kestrel/Infrastructure/MemoryPoolBlock2.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,5 +173,10 @@ public MemoryPoolIterator2 GetIterator()
{
return new MemoryPoolIterator2(this);
}

internal void Dispose()
{
GC.SuppressFinalize(this);
}
}
}
27 changes: 15 additions & 12 deletions test/Microsoft.AspNet.Server.KestrelTests/FrameTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}
82 changes: 44 additions & 38 deletions test/Microsoft.AspNet.Server.KestrelTests/MessageBodyTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,66 +19,72 @@ public class MessageBodyTests
[Fact]
public void Http10ConnectionClose()
{
var input = new TestInput();
var body = MessageBody.For("HTTP/1.0", new Dictionary<string, StringValues>(), input.FrameContext);
var stream = new FrameRequestStream(body);
using (var input = new TestInput())
{
var body = MessageBody.For("HTTP/1.0", new Dictionary<string, StringValues>(), 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<byte>(buffer1, 0, 5));
var buffer1 = new byte[1024];
var count1 = stream.Read(buffer1, 0, 1024);
AssertASCII("Hello", new ArraySegment<byte>(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<string, StringValues>(), input.FrameContext);
var stream = new FrameRequestStream(body);
using (var input = new TestInput())
{
var body = MessageBody.For("HTTP/1.0", new Dictionary<string, StringValues>(), 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<byte>(buffer1, 0, 5));
var buffer1 = new byte[1024];
var count1 = await stream.ReadAsync(buffer1, 0, 1024);
AssertASCII("Hello", new ArraySegment<byte>(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<string, StringValues>(), input.FrameContext);
var stream = new FrameRequestStream(body);
using (var input = new TestInput())
{
var body = MessageBody.For("HTTP/1.0", new Dictionary<string, StringValues>(), 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<byte>(readBuffer, 0, 8192));
var count1 = await stream.ReadAsync(readBuffer, 0, 8192);
Assert.Equal(8192, count1);
AssertASCII(largeInput, new ArraySegment<byte>(readBuffer, 0, 8192));

var count2 = await stream.ReadAsync(readBuffer, 0, 8192);
Assert.Equal(5, count2);
AssertASCII("Hello", new ArraySegment<byte>(readBuffer, 0, 5));
var count2 = await stream.ReadAsync(readBuffer, 0, 8192);
Assert.Equal(5, count2);
AssertASCII("Hello", new ArraySegment<byte>(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<byte> actual)
Expand Down
13 changes: 10 additions & 3 deletions test/Microsoft.AspNet.Server.KestrelTests/TestInput.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,28 @@

namespace Microsoft.AspNet.Server.KestrelTests
{
class TestInput : IConnectionControl, IFrameControl
class TestInput : IConnectionControl, IFrameControl, IDisposable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this require change MessageBodyTests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

{
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)
Expand Down