Skip to content

Comments

Improve dotnetup Muxer Handling, In-Use Muxer + Perf#52696

Merged
nagilson merged 13 commits intodotnet:release/dnupfrom
nagilson:nagilson-dotnetup-in-use-muxer
Feb 19, 2026
Merged

Improve dotnetup Muxer Handling, In-Use Muxer + Perf#52696
nagilson merged 13 commits intodotnet:release/dnupfrom
nagilson:nagilson-dotnetup-in-use-muxer

Conversation

@nagilson
Copy link
Member

@nagilson nagilson commented Jan 27, 2026

Fixes #51691
This also addresses the exception raised in this issue #51689

When moving the .NET SDK build to use dotnetup in nagilson#8, I hit several issues. The main issue is that the dotnet muxer may be in use when dotnetup tries to replace it, which would cause a failure.

I don't think we want to fail in this case and instead we want to emit a warning. I've added an option to fail instead, however.

What I also realized is that the approach of renaming the muxer even if it doesn't need to be replaced would cause the warning message to appear even if we didn't need to replace the muxer. Then I realized, we can do something clever. The windowsdesktop runtime has no runtime so we can skip muxer logic there. The other runtimes versions are the deciding factor for the muxer version since the muxer doesn't have its own version, so we can skip the resolution to find the version there.

For the SDK, what we can do to avoid an extra rename of the existing muxer if it doesn't need to be replaced, is to extract out the new muxer to a different name temporarily, and then look and see if the latest runtime in the dotnet install root target changed after that install. If so then the runtime that came with the SDK is newer so the new muxer should be used. This lets us gracefully handle when the muxer is in use as well as avoid any extra work we don't need to do.

#52696 this will fail quite often until this other PR is merged due to a dotnetup bug.

When moving the .NET SDK build to use `dotnetup`, I hit several issues. The main issue is that the dotnet muxer may be in use when dotnetup tries to replace it, which would cause a failure.

I don't think we want to fail in this case and instead we want to emit a warning. Maybe there should be an option to fail instead, however.

What I also realized is that the approach of renaming the muxer even if it doesn't need to be replaced would cause the warning message to appear even if we didn't need to replace the muxer.  Then I realized, we can do something clever. The windowsdesktop runtime has no runtime so we can skip muxer logic there. The other runtimes versions are the deciding factor for the muxer version since the muxer doesn't have its own version, so we can skip the resolution to find the version there. For the SDK, what we can do to avoid an extra rename of the existing muxer if it doesn't need to be replaced, is to extract out the new muxer to a different name temporarily, and then look and see if the latest runtime in the dotnet install root target changed after that install. If so then the runtime that came with  the SDK is newer so the new muxer should be used. This lets us gracefully handle when  the muxer is in use as well as avoid any extra work we don't need to do.
Copilot AI review requested due to automatic review settings February 9, 2026 18:34
Copy link
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 PR updates dotnetup’s install flow and the underlying installation library to handle muxer (dotnet/dotnet.exe) replacement more safely and with less unnecessary work—especially when the existing muxer is in use—while adding a switch to optionally fail instead of warning.

Changes:

  • Add --require-muxer-update and plumb it through dotnetup workflows into the installation library.
  • Refactor archive extraction to extract the muxer to a temp path first, then decide post-extraction whether to replace the installed muxer.
  • Add targeted tests covering muxer replacement decisions and “muxer in use” behavior.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test/dotnetup.Tests/MuxerHandlerTests.cs New unit tests for muxer replacement + locked-muxer scenarios
src/Installer/dotnetup/CommonOptions.cs Adds --require-muxer-update option
src/Installer/dotnetup/Commands/Shared/InstallWorkflow.cs Threads RequireMuxerUpdate through workflow options
src/Installer/dotnetup/Commands/Shared/InstallExecutor.cs Propagates RequireMuxerUpdate into install request options
src/Installer/dotnetup/Commands/Sdk/Install/SdkInstallCommandParser.cs Exposes + registers the new option for SDK installs
src/Installer/dotnetup/Commands/Sdk/Install/SdkInstallCommand.cs Passes RequireMuxerUpdate into the workflow
src/Installer/dotnetup/Commands/Runtime/Install/RuntimeInstallCommandParser.cs Registers the new option for runtime installs
src/Installer/dotnetup/Commands/Runtime/Install/RuntimeInstallCommand.cs Intended to pass RequireMuxerUpdate, but currently contains unresolved conflict
src/Installer/Microsoft.Dotnet.Installation/Microsoft.Dotnet.Installation.csproj Adds Spectre.Console dependency to installation library
src/Installer/Microsoft.Dotnet.Installation/Internal/MuxerHandler.cs New muxer handling logic (temp extract + conditional replace + in-use handling)
src/Installer/Microsoft.Dotnet.Installation/Internal/DotnetInstall.cs Adds RequireMuxerUpdate to install request options
src/Installer/Microsoft.Dotnet.Installation/Internal/DotnetArchiveExtractor.cs Integrates MuxerHandler into tar/zip extraction

@nagilson nagilson requested review from dsplaisted and removed request for dsplaisted February 9, 2026 22:26
Copy link
Member

@dsplaisted dsplaisted left a comment

Choose a reason for hiding this comment

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

Looks good.

The windowsdesktop runtime has no runtime so we can skip muxer logic there. The other runtimes versions are the deciding factor for the muxer version since the muxer doesn't have its own version, so we can skip the resolution to find the version there.

Is there any logic specific to whether we're installing an SDK or which shared framework we're installing? It seems like there's not (which seems like a good thing, it seems better just to base it off of the files we're extracting.

It's unfortunate that having a running dotnet app prevents the muxer from being overwritten. I kind of thought that it shouldn't work that way.

If we are going to fail it seems like it would be good to fail before we start extraction. We should be able to look at the archive and determine whether the muxer is newer than the existing version. So could our first operation be to move the existing muxer if we need to overwrite it? Then if we fail we won't have extracted any other files. That seems like a better experience, especially if you think we should change the default to fail if the muxer can't be overwritten.

@nagilson
Copy link
Member Author

It seems like there's not (which seems like a good thing, it seems better just to base it off of the files we're extracting.

I concur, there was at some point but I removed it so we would be flexible if the .zip format changed.

I like the idea of moving the muxer but I think this is risky with the lock implementations and race conditions. Instead we can simply ping to see if it's in use during prepare while we hold the lock.

@nagilson nagilson enabled auto-merge February 19, 2026 00:45
@dsplaisted
Copy link
Member

I like the idea of moving the muxer but I think this is risky with the lock implementations and race conditions. Instead we can simply ping to see if it's in use during prepare while we hold the lock.

@nagilson I'm not sure I get how it would be any more risky than what this PR does. It would be inside the lock, and I think the move is the same or less likely to fail than deleting it. It seems better to fail up-front rather than failing after we've already extracted everything.

@nagilson
Copy link
Member Author

Happy to change this, but my thinking is 2-fold:

  1. To properly check if the file is in use we need to hold an exclusive 'lock' on the file and thus it blocks other processes from using the muxer
  2. We separate actions where we hold the lock out - 'prepare' 'commit' and 'initialization' all pickup and drop a lock. This enables better concurrency. Moving the mutex early would prevent a more complex concurrency / mutex pattern. We would have to reimplement the mutex usage as it is now to make moving the muxer work.

I think the best approach for now is to check the muxer usage with the initial lock that sees if the install already exists, and then we will also fail elegantly if that changes between then and the commit action

@nagilson nagilson merged commit a903f93 into dotnet:release/dnup Feb 19, 2026
13 checks passed
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.

2 participants