Skip to content

Address deadlock and race conditions in WebSocketConnection - #55651

Open
tmat wants to merge 4 commits into
dotnet:mainfrom
tmat:AspireDeadlock
Open

Address deadlock and race conditions in WebSocketConnection#55651
tmat wants to merge 4 commits into
dotnet:mainfrom
tmat:AspireDeadlock

Conversation

@tmat

@tmat tmat commented Aug 6, 2026

Copy link
Copy Markdown
Member

Fixes deadlock #55560

Also update VS .NET TFM to net10

Copilot AI review requested due to automatic review settings August 6, 2026 21:54
@tmat
tmat requested a review from a team as a code owner August 6, 2026 21:54
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
2 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Aspire out-of-proc service (Dotnet.Watch/AspireService) to avoid a lock-based deadlock during WebSocket connection teardown, and bumps the Visual Studio service target framework to net10.0.

Changes:

  • Reworks SocketConnectionManager to use an ImmutableDictionary with interlocked updates instead of an explicit lock.
  • Refactors WebSocketConnection to a primary-constructor form (still IDisposable).
  • Simplifies shared-items compilation to include all *.cs under the AspireService directory and updates VS service TFM to net10.0.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/Dotnet.Watch/AspireService/Microsoft.WebTools.AspireService.projitems Switches from explicit file list to wildcard include for C# sources.
src/Dotnet.Watch/AspireService/Microsoft.WebTools.AspireService.Package.csproj Updates package comment; continues targeting $(VisualStudioServiceTargetFramework).
src/Dotnet.Watch/AspireService/Helpers/WebSocketConnection.cs Refactors the connection holder type (primary constructor) and keeps disposal behavior.
src/Dotnet.Watch/AspireService/Helpers/SocketConnectionManager.cs Replaces lock+Dictionary with lock-free immutable dictionary updates for connection tracking.
src/Dotnet.Watch/AspireService/Helpers/ImmutableInterlockedExtensions.cs Adds a helper for interlocked transform updates on immutable state.
Directory.Build.props Updates VisualStudioServiceTargetFramework to net10.0 and adds clarifying comment.
Suppressed comments (2)

src/Dotnet.Watch/AspireService/Helpers/SocketConnectionManager.cs:63

  • RemoveSocketConnection removes by key only. If a stale callback calls RemoveSocketConnection(oldConnection) after a new connection is registered with the same DcpId, ImmutableInterlocked.TryRemove will remove the new connection, then the code disposes the old instance. This can drop the active connection and leak/dispose the wrong instance.
    public void RemoveSocketConnection(WebSocketConnection connection)
    {
        // If the connection is not in the dictionary, then it has already been removed and disposed
        if (ImmutableInterlocked.TryRemove(ref _webSocketConnections, connection.DcpId, out var _))
        {

src/Dotnet.Watch/AspireService/Helpers/SocketConnectionManager.cs:69

  • _webSocketConnections is updated via interlocked operations but read without a volatile read. On weak memory models this can observe a stale dictionary snapshot and miss an added/removed connection. Using Volatile.Read aligns the read side with the lock-free publication pattern.
    public WebSocketConnection? GetSocketConnection(string dcpId)
        => _webSocketConnections.GetValueOrDefault(dcpId);

Comment thread src/Dotnet.Watch/AspireService/Helpers/SocketConnectionManager.cs
Comment thread src/Dotnet.Watch/AspireService/Helpers/WebSocketConnection.cs
Comment thread src/Dotnet.Watch/AspireService/Helpers/SocketConnectionManager.cs
Fixes deadlock dotnet#55560

Also update VS .NET TFM to net10
@tmat tmat changed the title Remove locking from WebSocketConnection. Address deadlock and race conditions in WebSocketConnection Aug 6, 2026
@tmat

tmat commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

@karolz-ms ptal

@karolz-ms karolz-ms left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM but please consider my questions regarding exception handling

Comment thread src/Dotnet.Watch/AspireService/Helpers/SocketConnectionManager.cs
Comment thread src/Dotnet.Watch/AspireService/Helpers/SocketConnectionManager.cs
@tmat
tmat enabled auto-merge (squash) August 7, 2026 16:58
@tmat

tmat commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).
2 pipeline(s) were filtered out due to trigger conditions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants