Skip to content

Improve shutdown sequence of Aspire service and fix race condition#53271

Merged
tmat merged 6 commits intodotnet:release/10.0.3xxfrom
tmat:WatchAspireMac
Mar 17, 2026
Merged

Improve shutdown sequence of Aspire service and fix race condition#53271
tmat merged 6 commits intodotnet:release/10.0.3xxfrom
tmat:WatchAspireMac

Conversation

@tmat
Copy link
Copy Markdown
Member

@tmat tmat commented Mar 5, 2026

Fixes intermittent test failure observed in #53424

Also fixes error reporting when expected exception is thrown after pipe disposal. This is a regression introduced by refactoring of named pipe transport

@tmat tmat requested a review from a team as a code owner March 5, 2026 01:11
Copilot AI review requested due to automatic review settings March 5, 2026 01:11
@tmat tmat requested review from a team as code owners March 5, 2026 01:11
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This is a large PR that refactors the dotnet-watch Aspire integration to support macOS (and other non-Windows platforms), introduces a new three-process architecture for Aspire watch (host, server, resource), and modernizes the test infrastructure. Previously, several Aspire-related tests were restricted to Windows via PlatformSpecificFact. The PR also introduces status reporting and control command pipes for communication between the Aspire host and watch server.

Changes:

  • Refactors Aspire watch into a three-process model (host, server, resource launchers) communicating via named pipes, replacing the old DotNetWatchLauncher/DotNetWatchOptions with command-based CLI (server, resource, host subcommands) and adds status/control pipe support.
  • Overhauls EnvironmentOptions to derive the muxer path from SdkDirectory instead of accepting it directly, adds a configurable LogMessagePrefix, introduces notification-only MessageDescriptors (LogLevel.None), and updates ProjectRepresentation to a record struct with OS-aware path equality.
  • Modernizes test infrastructure by replacing IDisposable with IAsyncDisposable, moving from BufferBlock to Channel<T> in AwaitableProcess, extracting TestEventObserver from TestReporter, splitting large test classes into focused files, and removing Windows-only test restrictions.

Reviewed changes

Copilot reviewed 93 out of 94 changed files in this pull request and generated 15 comments.

Show a summary per file
File Description
src/Dotnet.Watch/Watch/Context/EnvironmentOptions.cs Replace MuxerPath with SdkDirectory; derive muxer path from SDK dir
src/Dotnet.Watch/Watch/Context/EnvironmentVariables.cs Add ReadTimeSpanSeconds helper
src/Dotnet.Watch/Watch/Build/ProjectRepresentation.cs Convert to record struct with OS-aware equality on ProjectGraphPath
src/Dotnet.Watch/Watch/UI/IReporter.cs Add notification-only MessageDescriptors, configurable log prefix
src/Dotnet.Watch/Watch/UI/ConsoleReporter.cs Accept logMessagePrefix parameter
src/Dotnet.Watch/dotnet-watch/Program.cs Use new EnvironmentOptions API; accept ILoggerFactory
src/Dotnet.Watch/Watch/HotReload/HotReloadDotNetWatcher.cs Refactor build to emit notifications; centralize restart logic
src/Dotnet.Watch/Watch/HotReload/CompilationHandler.cs OS-aware path comparers; new RestartPeripheralProjectsAsync
src/Dotnet.Watch/Watch/Process/RunningProject.cs Change RestartOperation to ValueTask; add RestartAsync
src/Dotnet.Watch/Watch/Process/ProjectLauncher.cs Expose CompilationHandler; use GetMuxerPath()
src/Dotnet.Watch/Watch/Aspire/AspireServiceFactory.cs Return ValueTask from StartProjectAsync
src/Dotnet.Watch/Watch.Aspire/AspireLauncher.cs New base class for Aspire launchers with CLI parsing
src/Dotnet.Watch/Watch.Aspire/Server/AspireServerLauncher.cs New server launcher with status/control pipes
src/Dotnet.Watch/Watch.Aspire/Server/ProcessLauncherFactory.cs Named pipe listener accepting resource launch requests
src/Dotnet.Watch/Watch.Aspire/Server/WatchStatusWriter.cs Writes status events to named pipe
src/Dotnet.Watch/Watch.Aspire/Server/WatchControlReader.cs Reads control commands (e.g., rebuild) from pipe
src/Dotnet.Watch/Watch.Aspire/Server/StatusReportingLoggerFactory.cs Intercepts log events for status reporting
src/Dotnet.Watch/Watch.Aspire/Resource/AspireResourceLauncher.cs Resource process connecting to server pipe
src/Dotnet.Watch/Watch.Aspire/Host/AspireHostLauncher.cs Host launcher for single-project watch
src/Dotnet.Watch/Watch.Aspire/Commands/*.cs CLI command definitions for server, resource, host
src/Dotnet.Watch/Watch.Aspire/Program.cs Rewritten entry point dispatching to launcher
test/.../AwaitableProcess.cs Replace BufferBlock with Channel<T>; async dispose
test/.../WatchableApp.cs Generic launcher; async dispose; configurable executable
test/.../DotNetWatchTestBase.cs IAsyncLifetime; CreateInProcWatcher factory
test/.../TestEventObserver.cs Extracted event observation from TestReporter
Various test files Split tests into focused files; remove Windows-only restrictions
test/.../AspireLauncherTests.cs New integration test for server/resource communication
test/.../Aspire*CliTests.cs CLI parsing tests for all three subcommands
src/Dotnet.Watch/.editorconfig Suppress diagnostics for Watch solution

Comment thread src/Dotnet.Watch/Watch.Aspire/Server/WatchStatusWriter.cs Outdated
Comment thread src/Dotnet.Watch/Watch.Aspire/Server/ProcessLauncherFactory.cs
Comment thread src/Dotnet.Watch/Watch.Aspire/Utilities/AspireEnvironmentVariables.cs Outdated
Comment thread test/dotnet-watch.Tests/TestUtilities/TestReporter.cs
Comment thread test/Microsoft.DotNet.HotReload.Test.Utilities/AwaitableProcess.cs
Comment thread test/Microsoft.DotNet.HotReload.Watch.Aspire.Tests/AspireLauncherTests.cs Outdated
Comment thread src/Dotnet.Watch/Watch.Aspire/Server/AspireWatcherLauncher.cs Outdated
Comment thread src/Dotnet.Watch/Watch/Build/ProjectRepresentation.cs
@tmat tmat requested a review from a team as a code owner March 15, 2026 22:07
@tmat tmat changed the title Watch aspire mac Improve shutdown sequence of Aspire service and fix race condition Mar 15, 2026
@tmat
Copy link
Copy Markdown
Member Author

tmat commented Mar 16, 2026

@DustinCampbell ptal
@mmitche FYI

Comment thread src/Dotnet.Watch/Watch/Aspire/AspireServiceFactory.cs Outdated
Comment thread test/dotnet-watch.Tests/HotReload/AspireHotReloadTests.cs
Comment thread src/Dotnet.Watch/Watch/Aspire/AspireServiceFactory.cs
@tmat tmat merged commit e0a6b80 into dotnet:release/10.0.3xx Mar 17, 2026
28 checks passed
@tmat tmat deleted the WatchAspireMac branch March 17, 2026 16:20
mmitche added a commit that referenced this pull request Mar 17, 2026
The dotnet-watch Aspire race condition fix from this PR has been superseded
by a proper fix in #53271 (merged to main). Reverting these files
so the merge from main brings in the better fix.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mmitche added a commit that referenced this pull request Mar 17, 2026
…ther updates

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mmitche added a commit that referenced this pull request Mar 17, 2026
Reverted dotnet-watch changes (superseded by tmat's fix in #53271).
Merged from main to pick up latest changes.
Starting fresh validation run toward 25 consecutive passes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DonnaChen888 pushed a commit that referenced this pull request Mar 18, 2026
…53271)

Co-authored-by: Matt Mitchell <mmitche@microsoft.com>
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.

4 participants