Skip to content

Commit

Permalink
Fix ManagedWebSocket ordering of releasing send buffer and semaphore …
Browse files Browse the repository at this point in the history
…(#39199)

Once we release the semaphore, we no longer have ownership over _sendBuffer, so we have to release the latter before not after releasing the semaphore.
  • Loading branch information
stephentoub authored Jul 14, 2020
1 parent 8892364 commit 2287fe5
Showing 1 changed file with 8 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode,
// If we get here, the cancellation token is not cancelable so we don't have to worry about it,
// and we own the semaphore, so we don't need to asynchronously wait for it.
ValueTask writeTask = default;
bool releaseSemaphoreAndSendBuffer = true;
bool releaseSendBufferAndSemaphore = true;
try
{
// Write the payload synchronously to the buffer, then write that buffer out to the network.
Expand All @@ -402,7 +402,7 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode,
// Up until this point, if an exception occurred (such as when accessing _stream or when
// calling GetResult), we want to release the semaphore and the send buffer. After this point,
// both need to be held until writeTask completes.
releaseSemaphoreAndSendBuffer = false;
releaseSendBufferAndSemaphore = false;
}
catch (Exception exc)
{
Expand All @@ -413,10 +413,10 @@ private ValueTask SendFrameLockAcquiredNonCancelableAsync(MessageOpcode opcode,
}
finally
{
if (releaseSemaphoreAndSendBuffer)
if (releaseSendBufferAndSemaphore)
{
_sendFrameAsyncLock.Release();
ReleaseSendBuffer();
_sendFrameAsyncLock.Release();
}
}

Expand All @@ -437,8 +437,8 @@ private async ValueTask WaitForWriteTaskAsync(ValueTask writeTask)
}
finally
{
_sendFrameAsyncLock.Release();
ReleaseSendBuffer();
_sendFrameAsyncLock.Release();
}
}

Expand All @@ -461,8 +461,8 @@ private async ValueTask SendFrameFallbackAsync(MessageOpcode opcode, bool endOfM
}
finally
{
_sendFrameAsyncLock.Release();
ReleaseSendBuffer();
_sendFrameAsyncLock.Release();
}
}

Expand Down Expand Up @@ -1277,6 +1277,8 @@ private void AllocateSendBuffer(int minLength)
/// <summary>Releases the send buffer to the pool.</summary>
private void ReleaseSendBuffer()
{
Debug.Assert(_sendFrameAsyncLock.CurrentCount == 0, "Caller should hold the _sendFrameAsyncLock");

byte[]? old = _sendBuffer;
if (old != null)
{
Expand Down

0 comments on commit 2287fe5

Please sign in to comment.