Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,13 @@ private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus,
{
_closeReceivedTaskCompletionSource ??= new TaskCompletionSource();

// _thisLock MUST be released before starting the CloseOutput operation: it acquires the
// SessionHandle-lock, which MUST always be acquired before _thisLock (see TakeLocks).
// Acquiring the locks in the opposite order here would deadlock with a thread that is
// processing a received close frame, as that one holds the SessionHandle-lock while
// acquiring _thisLock in StartOnCloseReceived.
ReleaseLock(_thisLock, ref lockTaken);

closeOutputTask = CloseOutputAsync(closeStatus,
statusDescription,
linkedCancellationToken);
Expand Down Expand Up @@ -716,9 +723,13 @@ private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus,
ArraySegment<byte> closeMessageBuffer =
new ArraySegment<byte>(new byte[HttpWebSocket.MinReceiveBufferSize]);
EnsureReceiveOperation();

// As above, _thisLock MUST be released before starting the receive operation, because
// WebSocketOperation.Process acquires the SessionHandle-lock.
ReleaseLock(_thisLock, ref lockTaken);

Task<WebSocketReceiveResult?> receiveAsyncTask = _receiveOperation!.Process(closeMessageBuffer,
linkedCancellationToken);
ReleaseLock(_thisLock, ref lockTaken);

WebSocketReceiveResult? receiveResult = null;
try
Expand Down Expand Up @@ -816,6 +827,11 @@ private async Task CloseAsyncCore(WebSocketCloseStatus closeStatus,
catch (Exception exception)
{
bool aborted = linkedCancellationToken.IsCancellationRequested;

// As above, _thisLock MUST be released before calling Abort, because it acquires the
// SessionHandle-lock.
ReleaseLock(_thisLock, ref lockTaken);

Abort();
ThrowIfConvertibleException(nameof(CloseAsync), exception, cancellationToken, aborted);
throw;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,52 @@ public async Task Abort_CallAfterAborted_Nop()
Assert.Equal(WebSocketState.Aborted, context.WebSocket.State);
}

[ConditionalFact(typeof(Helpers), nameof(Helpers.IsWindowsImplementation))]
public async Task CloseAsync_ConcurrentWithCloseFrameFromClient_DoesNotDeadlock()
{
// Closing a WebSocket from both ends at the same time used to deadlock: the thread processing
// an incoming close frame holds the session handle lock while acquiring the state lock, while
// CloseAsync held the state lock while acquiring the session handle lock.
const int Iterations = 100;
Random random = new Random(42);

for (int i = 0; i < Iterations; i++)
{
using ClientWebSocket client = new ClientWebSocket();
HttpListenerWebSocketContext context = await GetWebSocketContext(client);
WebSocket server = context.WebSocket;

// The pending receive makes a separate thread process the close frame sent by the client.
Task serverReceiveTask = IgnoreExpectedExceptionsAsync(
server.ReceiveAsync(new ArraySegment<byte>(new byte[16]), CancellationToken.None));

Task clientCloseTask = IgnoreExpectedExceptionsAsync(
client.CloseOutputAsync(WebSocketCloseStatus.NormalClosure, null, CancellationToken.None));

// Sweep the (very short) window in which the deadlock can happen.
Thread.SpinWait(random.Next(200_000));

Task serverCloseTask = IgnoreExpectedExceptionsAsync(Task.Run(
() => server.CloseAsync(WebSocketCloseStatus.NormalClosure, null, CancellationToken.None)));

Task allTasks = Task.WhenAll(serverReceiveTask, clientCloseTask, serverCloseTask);
await allTasks.WaitAsync(TimeSpan.FromSeconds(30));

server.Dispose();
}

static async Task IgnoreExpectedExceptionsAsync(Task task)
{
try
{
await task;
}
catch (Exception e) when (e is WebSocketException or InvalidOperationException or ObjectDisposedException or OperationCanceledException)
{
}
}
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsWindows))]
public async Task ReceiveAsync_ReadBuffer_WithWindowsAuthScheme_Success()
{
Expand Down Expand Up @@ -430,5 +476,23 @@ private async Task<HttpListenerWebSocketContext> GetWebSocketContext(string[] su
HttpListenerContext context = await serverContextTask;
return await context.AcceptWebSocketAsync(null);
}

private async Task<HttpListenerWebSocketContext> GetWebSocketContext(ClientWebSocket client)
{
var uriBuilder = new UriBuilder(Factory.ListeningUrl) { Scheme = "ws" };
Task<HttpListenerContext> serverContextTask = Factory.GetListener().GetContextAsync();

Task clientConnectTask = client.ConnectAsync(uriBuilder.Uri, CancellationToken.None);
if (clientConnectTask == await Task.WhenAny(serverContextTask, clientConnectTask))
{
await clientConnectTask;
Assert.Fail("Client should not have completed prior to server sending response");
}

HttpListenerContext context = await serverContextTask;
HttpListenerWebSocketContext webSocketContext = await context.AcceptWebSocketAsync(null);
await clientConnectTask;
return webSocketContext;
}
}
}