Remove race condition from socket input that could stall reads

- Without the _sync lock, if new data was produced as ConsumingComplete
  was called, the next "await SocketInput" might never complete despite not
  all data being examined.
- If more data is produced afterward, the stall would be prevented, but this
  isn't always the case such as during the end of the request.
This commit is contained in:
Stephen Halter 2016-03-10 09:14:36 -08:00
parent 850632a091
commit 84a68208d0
1 changed files with 82 additions and 72 deletions

View File

@ -28,6 +28,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http
private MemoryPoolBlock _pinned; private MemoryPoolBlock _pinned;
private int _consumingState; private int _consumingState;
private object _sync = new object();
public SocketInput(MemoryPool memory, IThreadPool threadPool) public SocketInput(MemoryPool memory, IThreadPool threadPool)
{ {
@ -58,64 +59,70 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http
public void IncomingData(byte[] buffer, int offset, int count) public void IncomingData(byte[] buffer, int offset, int count)
{ {
if (count > 0) lock (_sync)
{ {
if (_tail == null) if (count > 0)
{ {
_tail = _memory.Lease(); if (_tail == null)
{
_tail = _memory.Lease();
}
var iterator = new MemoryPoolIterator(_tail, _tail.End);
iterator.CopyFrom(buffer, offset, count);
if (_head == null)
{
_head = _tail;
}
_tail = iterator.Block;
}
else
{
RemoteIntakeFin = true;
} }
var iterator = new MemoryPoolIterator(_tail, _tail.End); Complete();
iterator.CopyFrom(buffer, offset, count);
if (_head == null)
{
_head = _tail;
}
_tail = iterator.Block;
} }
else
{
RemoteIntakeFin = true;
}
Complete();
} }
public void IncomingComplete(int count, Exception error) public void IncomingComplete(int count, Exception error)
{ {
if (_pinned != null) lock (_sync)
{ {
_pinned.End += count; if (_pinned != null)
{
_pinned.End += count;
if (_head == null) if (_head == null)
{ {
_head = _tail = _pinned; _head = _tail = _pinned;
} }
else if (_tail == _pinned) else if (_tail == _pinned)
{ {
// NO-OP: this was a read into unoccupied tail-space // NO-OP: this was a read into unoccupied tail-space
} }
else else
{ {
_tail.Next = _pinned; _tail.Next = _pinned;
_tail = _pinned; _tail = _pinned;
}
_pinned = null;
} }
_pinned = null; if (count == 0)
} {
RemoteIntakeFin = true;
}
if (error != null)
{
_awaitableError = error;
}
if (count == 0) Complete();
{
RemoteIntakeFin = true;
} }
if (error != null)
{
_awaitableError = error;
}
Complete();
} }
public void IncomingDeferred() public void IncomingDeferred()
@ -162,40 +169,43 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Http
MemoryPoolIterator consumed, MemoryPoolIterator consumed,
MemoryPoolIterator examined) MemoryPoolIterator examined)
{ {
MemoryPoolBlock returnStart = null; lock (_sync)
MemoryPoolBlock returnEnd = null;
if (!consumed.IsDefault)
{ {
returnStart = _head; MemoryPoolBlock returnStart = null;
returnEnd = consumed.Block; MemoryPoolBlock returnEnd = null;
_head = consumed.Block;
_head.Start = consumed.Index;
}
if (!examined.IsDefault && if (!consumed.IsDefault)
examined.IsEnd && {
RemoteIntakeFin == false && returnStart = _head;
_awaitableError == null) returnEnd = consumed.Block;
{ _head = consumed.Block;
_manualResetEvent.Reset(); _head.Start = consumed.Index;
}
Interlocked.CompareExchange( if (!examined.IsDefault &&
ref _awaitableState, examined.IsEnd &&
_awaitableIsNotCompleted, RemoteIntakeFin == false &&
_awaitableIsCompleted); _awaitableError == null)
} {
_manualResetEvent.Reset();
while (returnStart != returnEnd) Interlocked.CompareExchange(
{ ref _awaitableState,
var returnBlock = returnStart; _awaitableIsNotCompleted,
returnStart = returnStart.Next; _awaitableIsCompleted);
returnBlock.Pool.Return(returnBlock); }
}
if (Interlocked.CompareExchange(ref _consumingState, 0, 1) != 1) while (returnStart != returnEnd)
{ {
throw new InvalidOperationException("No ongoing consuming operation to complete."); var returnBlock = returnStart;
returnStart = returnStart.Next;
returnBlock.Pool.Return(returnBlock);
}
if (Interlocked.CompareExchange(ref _consumingState, 0, 1) != 1)
{
throw new InvalidOperationException("No ongoing consuming operation to complete.");
}
} }
} }