Skip to content

Fix daemon relay exit classification - #84666

Merged
dibarbet merged 7 commits into
dotnet:mainfrom
dibarbet:dibarbet-fix-daemon-relay-exit-code
Jul 29, 2026
Merged

dibarbet merged 7 commits into
dotnet:mainfrom
dibarbet:dibarbet-fix-daemon-relay-exit-code

Conversation

@dibarbet

@dibarbet dibarbet commented Jul 28, 2026

Copy link
Copy Markdown
Member

Fixes #84662

When the daemon endpoint is lost, both directional relay tasks can complete against the server endpoint. The thin client previously interpreted any pair of completed relay tasks as a clean shutdown and returned exit code 0.

Classify relay completion using the endpoint reported by each copy direction:

  • two server-endpoint completions indicate daemon connection loss
  • a single completion indicates loss of that endpoint
  • any other pair indicates clean shutdown

A clean editor shutdown may complete both copies at the editor because the editor closes its bidirectional transport after sending LSP exit, before the daemon closes its side.

Validation:

  • Built src\LanguageServer\roslyn-language-server\roslyn-language-server.csproj
  • Ran the existing daemon lifecycle tests for clean shutdown, daemon loss, editor loss, and keepalive behavior across both transports (7 passed)
  • Ran KeepAlive_DaemonReusedWithinWindow_ThenExitsWhenIdle in Release (passed)
Microsoft Reviewers: Open in CodeFlow

Classify relay completion by the endpoint that caused each copy direction to stop, so a lost daemon or editor cannot be mistaken for a clean shutdown.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f809f59b-563b-4a70-b5ea-df3af70d5abd
Copilot AI review requested due to automatic review settings July 28, 2026 20:16
@dibarbet
dibarbet requested a review from a team as a code owner July 28, 2026 20:16
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 2 pipeline(s).
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 roslyn-language-server thin client’s daemon relay shutdown logic so it can reliably distinguish a clean LSP shutdown from loss of either the editor or daemon endpoint, avoiding incorrectly returning exit code 0 when the daemon is killed.

Changes:

  • Replace the relay “both sides closed” heuristic with an explicit RelayCompletionKind classification.
  • Determine clean shutdown vs. editor/daemon connection loss by comparing the endpoints reported by each relay direction.
Show a summary per file
File Description
src/LanguageServer/roslyn-language-server/Program.cs Consumes the new relay completion classification to choose the correct exit code (success vs. editor/daemon lost).
src/LanguageServer/roslyn-language-server/LspRelay.cs Introduces RelayCompletionKind and updates relay completion logic to classify shutdown based on which endpoint each copy direction reports.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f809f59b-563b-4a70-b5ea-df3af70d5abd
Copilot AI review requested due to automatic review settings July 28, 2026 21:47
dibarbet added 4 commits July 28, 2026 14:48
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f809f59b-563b-4a70-b5ea-df3af70d5abd

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.

Copilot's findings

Comments suppressed due to low confidence (1)

src/LanguageServer/roslyn-language-server/LspRelay.cs:41

  • RelayAsync now awaits completedTask before canceling the other copy direction. If CopyUntilClosedAsync faults unexpectedly, this will throw before cancellationSource.Cancel() runs, potentially leaving the other copy task running and holding resources/streams open. Wrapping the awaits in a try/finally (or deferring the await until after cancel) keeps the previous guarantee that the non-winning direction is always canceled.
        var completedTask = await Task.WhenAny(editorToServer, serverToEditor).ConfigureAwait(false);
        var closedEndpoint = await completedTask.ConfigureAwait(false);

        // Give the other direction a brief window to finish on its own. If both copies terminate at the server,
        // the server connection was lost and caused both directions to stop. Any other pair is a clean shutdown:
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new

Copilot AI review requested due to automatic review settings July 28, 2026 21:53

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.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new

Comment thread src/LanguageServer/roslyn-language-server/LspRelay.cs Outdated
Copilot AI review requested due to automatic review settings July 28, 2026 22:41

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.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new

@dibarbet
dibarbet merged commit 24d1db3 into dotnet:main Jul 29, 2026
25 checks passed
@dibarbet
dibarbet deleted the dibarbet-fix-daemon-relay-exit-code branch July 29, 2026 00:00
GoldMikeDev pushed a commit to GoldMikeDev/FollyOfTheUnbound that referenced this pull request Jul 29, 2026
Follow-up to the origin/main merge: our fork's LspRelay.RelayAsync classifier
was merged with upstream's own recent fix for the same class of bug
(dotnet#84666, "Fix daemon relay exit classification") -- keeping our
fork's more complete algorithm (graceful/ungraceful tracking, so a server
that closes cleanly without the editor also closing is still classified as
clean, which upstream's own fix doesn't cover) under upstream's cleaner
RelayCompletionKind enum API. Updated this fork's own LspRelay tests to
match the merged signature; all 4 still pass with the same semantics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DaemonServerLifecycleTests.DaemonKilled_EveryThinClientExitsNonZero fails intermittently

4 participants