Skip to content

[dotnet-watch] Notify DCP of terminated session when process exits on its own #50992

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Sep 30, 2025
Merged

[dotnet-watch] Notify DCP of terminated session when process exits on its own #50992

merged 2 commits into from
Sep 30, 2025

Conversation

tmat
Copy link
Member

@tmat tmat commented Sep 24, 2025 •
edited
Loading

Description

Previously we only notified DCP when dotnet-watch terminated the process.

Fixes cancellation handling in a few places to avoid reporting errors that are caused by expected process termination/shutdown and pass correct cancellation tokens.
Fixes process output redirection for Aspire child processes.

Fixes #50952
Fixes dotnet/aspire#9756

Customer Impact

Impacts Aspire customers.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Justification: Only impacts dotnet-watch and Aspire CLI.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@github-actions GitHub Actions
Copy link
Contributor

This PR is targeting main, which is now for .NET 11-facing work. If you intended to target .NET 10, either retarget this PR to release/10.0.1xx or make sure you backport the change to release/10.0.1xx after merging. See #50394 for more details.

@tmat tmat changed the base branch from main to release/10.0.1xx September 24, 2025 23:50
@github-actions github-actions bot added the Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch label Sep 24, 2025
@dotnet-policy-service Dotnet Policy Service
Copy link
Contributor

Thanks for your PR, @@tmat.
To learn about the PR process and branching schedule of this repo, please take a look at the SDK PR Guide.

@tmat tmat force-pushed the SessionTerminated branch from 3bca85d to 85bf7bf Compare September 25, 2025 00:43
@tmat tmat marked this pull request as ready for review September 25, 2025 00:43
@tmat tmat requested review from a team as code owners September 25, 2025 00:43
@tmat tmat force-pushed the SessionTerminated branch from 85bf7bf to 1dd6d3b Compare September 25, 2025 17:50
@tmat tmat changed the title Session terminated Notify DCP of terminated session when process exits on its own Sep 25, 2025
@tmat tmat marked this pull request as draft September 25, 2025 19:13
@tmat tmat force-pushed the SessionTerminated branch from 1dd6d3b to fc261e5 Compare September 25, 2025 19:35
@tmat tmat marked this pull request as ready for review September 25, 2025 19:35
This was referenced Sep 25, 2025
Closed
Open
Open
@tmat tmat marked this pull request as draft September 25, 2025 20:42
@tmat tmat force-pushed the SessionTerminated branch from 5dc2950 to 4fa949d Compare September 26, 2025 04:46
@build-analysis build-analysis bot mentioned this pull request Sep 26, 2025
Open
@tmat tmat force-pushed the SessionTerminated branch from 6471a0c to b942ea0 Compare September 26, 2025 18:29
@tmat tmat marked this pull request as ready for review September 26, 2025 20:32
@tmat tmat force-pushed the SessionTerminated branch from b942ea0 to 0dd4e52 Compare September 26, 2025 21:13
@tmat
Copy link
Member Author

tmat commented Sep 26, 2025

@DustinCampbell ptal

@tmat tmat added Area-Watch and removed Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch labels Sep 26, 2025
@tmat tmat added this to the 10.0.1xx milestone Sep 26, 2025
@tmat tmat changed the title Notify DCP of terminated session when process exits on its own [Hot Reload] Notify DCP of terminated session when process exits on its own Sep 26, 2025
@tmat tmat changed the title [Hot Reload] Notify DCP of terminated session when process exits on its own [dotnet-watch] Notify DCP of terminated session when process exits on its own Sep 26, 2025
DustinCampbell
DustinCampbell approved these changes Sep 29, 2025
View reviewed changes
Copy link
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

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

I read through everything and didn't spot any cases where cancellation tokens and disposable objects didn't flow as they did before.

src/BuiltInTools/dotnet-watch/HotReload/HotReloadDotNetWatcher.cs
await Task.WhenAll(
projectsToRestart.Select(async runningProject =>
{
var newRunningProject = await runningProject.RestartOperation(shutdownCancellationToken);
Copy link
Member

Choose a reason for hiding this comment

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

Does RestartOperation need an "Async" suffix?

test/dotnet-watch.Tests/TestUtilities/AssertEx.cs

var message = new StringBuilder();
message.AppendLine($"Expected output {(expectedPresent ? "not found" : "found")}:");

Copy link
Member

Choose a reason for hiding this comment

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

nit: extra blank line

@build-analysis build-analysis bot mentioned this pull request Sep 30, 2025
Open
@marcpopMSFT
Copy link
Member

approved via email, failure is a known issue.

@marcpopMSFT marcpopMSFT merged commit 5b52129 into dotnet:release/10.0.1xx Sep 30, 2025
27 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DustinCampbell DustinCampbell DustinCampbell approved these changes

Assignees

No one assigned

Labels

Area-Watch Servicing-approved

Projects

None yet

Milestone

10.0.1xx

Development

Successfully merging this pull request may close these issues.

3 participants

@tmat @marcpopMSFT @DustinCampbell