Getting System.NullReferenceException when running `dotnet tool update -g --all`

voroninp commented 1 month ago

Description

When I run dotnet tool update -g --all CLI fails with this exception:

Unhandled exception: System.NullReferenceException: Object reference not set to an instance of an object. at Microsoft.DotNet.Tools.Tool.List.ToolListGlobalOrToolPathCommand.GetPackages(Nullable1 toolPath, Nullable1 packageId) at Microsoft.DotNet.Tools.Tool.Install.ToolInstallGlobalOrToolPathCommand.Execute() at Microsoft.DotNet.Tools.Tool.Update.ToolUpdateGlobalOrToolPathCommand.Execute() at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult parseResult) at System.CommandLine.ParseResult.Invoke() at Microsoft.DotNet.Cli.Program.ProcessArgs(String[] args, TimeSpan startupTime, ITelemetry telemetryClient)

Reproduction Steps

I am not sure, because when I run this command under 8.0.303 I get the following error:

--all is not found in NuGet feeds https://api.nuget.org/v3/index.json".

I assume it's a new feature, because when I do not specify --all for .NET 9 preview, I get this message:

One must specify either package ID or use the update all option (--all).

I also could not find --all here

Expected behavior

Either runs cucessfully or properly communicates the reason for failure

Actual behavior

Miserably fails.

Regression?

I assume so.

Known Workarounds

No response

Configuration

Here's the output of dotnet --info:

.NET SDK:
 Version:           9.0.100-preview.6.24328.19
 Commit:            ef4c241666
 Workload version:  9.0.100-manifests.21d7f649
 MSBuild version:   17.11.0-preview-24318-05+4a45d5633

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22631
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\9.0.100-preview.6.24328.19\

.NET workloads installed:
Configured to use loose manifests when installing new manifests.
 [android]
   Installation Source: SDK 9.0.100-preview.6, VS 17.11.35201.85, VS 17.10.35122.118
   Manifest Version:    34.99.0-preview.6.340/9.0.100-preview.6
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.6\microsoft.net.sdk.android\34.99.0-preview.6.340\WorkloadManifest.json
   Install Type:              Msi

 [aspire]
   Installation Source: SDK 9.0.100-preview.6, VS 17.10.35122.118
   Manifest Version:    8.1.0/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.aspire\8.1.0\WorkloadManifest.json
   Install Type:        FileBased

 [ios]
   Installation Source: SDK 9.0.100-preview.6, VS 17.11.35201.85, VS 17.10.35122.118
   Manifest Version:    17.2.9714-net9-p6/9.0.100-preview.6
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.6\microsoft.net.sdk.ios\17.2.9714-net9-p6\WorkloadManifest.json
   Install Type:              Msi

 [maccatalyst]
   Installation Source: SDK 9.0.100-preview.6, VS 17.11.35201.85, VS 17.10.35122.118
   Manifest Version:    17.2.9714-net9-p6/9.0.100-preview.6
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.6\microsoft.net.sdk.maccatalyst\17.2.9714-net9-p6\WorkloadManifest.json
   Install Type:              Msi

 [maui-windows]
   Installation Source: SDK 9.0.100-preview.6, VS 17.11.35201.85, VS 17.10.35122.118
   Manifest Version:    9.0.0-preview.6.24327.7/9.0.100-preview.6
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.6\microsoft.net.sdk.maui\9.0.0-preview.6.24327.7\WorkloadManifest.json
   Install Type:              Msi

 [wasm-tools]
   Installation Source: SDK 9.0.100-preview.6, VS 17.11.35201.85, VS 17.10.35122.118
   Manifest Version:    9.0.0-preview.6.24327.7/9.0.100-preview.6
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\9.0.100-preview.6\microsoft.net.workload.mono.toolchain.current\9.0.0-preview.6.24327.7\WorkloadManifest.json
   Install Type:              Msi

Host:
  Version:      9.0.0-preview.6.24327.7
  Architecture: x64
  Commit:       static

.NET SDKs installed:
  7.0.410 [C:\Program Files\dotnet\sdk]
  8.0.303 [C:\Program Files\dotnet\sdk]
  8.0.400-preview.0.24324.5 [C:\Program Files\dotnet\sdk]
  9.0.100-preview.6.24328.19 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.32 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.19 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 9.0.0-preview.6.24328.4 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.32 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.19 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 9.0.0-preview.6.24327.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.30 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.32 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.19 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 9.0.0-preview.6.24327.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Other information

Here's the output of dotnet tool list -g:

Package Id                             Version         Commands
------------------------------------------------------------------------
csharprepl                             0.6.5           csharprepl
dotnet-depends                         0.7.0           dotnet-depends
dotnet-ef                              8.0.7           dotnet-ef
dotnet-grpc                            2.53.0          dotnet-grpc
dotnet-nugetize                        1.0.4           nugetize
dotnet-reportgenerator-globaltool      5.1.10          reportgenerator
dotnet-svcutil                         2.1.0           dotnet-svcutil
microsoft.dotnet-httprepl              8.0.0           httprepl
microsoft.dotnet.apicompat.tool        8.0.204         apicompat
nuke.globaltool                        6.2.1           nuke
upgrade-assistant                      0.4.421302      upgrade-assistant

ralish commented 1 month ago

Can confirm this also occurs in the just released .NET 8.0.400, which is the first stable version to include the --all parameter. The issue which tracked the introduction of the new parameter is #10130 and PR is #38996.

Example output:

 ~#@❯ dotnet tool list --global
Package Id                           Version         Commands
---------------------------------------------------------------------
csharpier                            0.28.2          dotnet-csharpier
dotnet-counters                      8.0.532401      dotnet-counters
dotnet-coverage                      17.11.3         dotnet-coverage
dotnet-dump                          8.0.532401      dotnet-dump
dotnet-gcdump                        8.0.532401      dotnet-gcdump
dotnet-monitor                       8.0.3           dotnet-monitor
dotnet-outdated-tool                 4.6.4           dotnet-outdated
dotnet-stack                         8.0.532401      dotnet-stack
dotnet-suggest                       1.1.415701      dotnet-suggest
dotnet-symbol                        8.0.532401      dotnet-symbol
dotnet-trace                         8.0.532401      dotnet-trace
jetbrains.resharper.globaltools      2024.1.4        jb
microsoft.cst.devskim.cli            1.0.33          devskim
microsoft.sbom.dotnettool            2.2.6           sbom-tool
wix                                  5.0.0           wix
 ~#@❯ dotnet tool update --global
One must specify either package ID or use the update all option (--all).
 ~#@❯ dotnet tool update --global --all
Unhandled exception: System.NullReferenceException: Object reference not set to an instance of an object.
   at Microsoft.DotNet.Tools.Tool.List.ToolListGlobalOrToolPathCommand.GetPackages(Nullable`1 toolPath, Nullable`1 packageId)
   at Microsoft.DotNet.Tools.Tool.Install.ToolInstallGlobalOrToolPathCommand.Execute()
   at Microsoft.DotNet.Tools.Tool.Update.ToolUpdateGlobalOrToolPathCommand.Execute()
   at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult parseResult)
   at Microsoft.DotNet.Cli.Program.ProcessArgs(String[] args, TimeSpan startupTime, ITelemetry telemetryClient)

WeihanLi commented 2 weeks ago

still broken in .NET 9 Preview 7

DL444 commented 2 weeks ago

Analysis

The problem is at ToolListGlobalOrToolPathCommand.cs:87, where _createToolPackageStore(toolPath) returns null, causing chaining calls to throw NullReferenceException.

This is eventually traced back to ToolInstallGlobalOrToolPathCommand.cs:123, where ToolListGlobalOrToolPathCommand constructor is called with argument toolPath => { return _store; } for parameter createToolPackageStore. This is used to initialize the _createToolPackageStore delegate that caused troubles as described earlier.

_store is definitely null in this scenario. This is assigned by the constructor of ToolInstallGlobalOrToolPathCommand, which in this case is called at ToolUpdateCommand.cs:48 using the default parameter value null for _store.

After reading the implementation I came to believe that _store being null is expected here for global tool updates. I reached this hypothesis based on the following observations:

The constructor of the concrete store implementation class (ToolPackageStoreAndQuery) takes an optional parameter nonGlobalLocation where null means global package store. This suggests that using null for global store is established practice.
Store being null is somewhat guarded by the ToolListGlobalOrToolPathCommand constructor, which assigns _createToolPackageStore to a factory method that creates ToolPackageStoreAndQuery for the global store if the parameter is null.

Unfortunately, the guard outlined by the second bullet is not effective, because as mentioned in the second paragraph, the argument itself is not null. Instead, it's what the delegate argument returns when called that is null.

Code Reference

// ToolListGlobalOrToolPathCommand.cs
public ToolListGlobalOrToolPathCommand(
    ParseResult result,
    CreateToolPackageStore createToolPackageStore = null,
    IReporter reporter = null)
    : base(result)
{
    // ...
    // _createToolPackageStore assigned here.
    // Notes that it only guards `createToolPackageStore` being null, not `createToolPackageStore()` being null.
    _createToolPackageStore = createToolPackageStore ?? ToolPackageFactory.CreateToolPackageStoreQuery;
}

public IEnumerable<IToolPackage> GetPackages(DirectoryPath? toolPath, PackageId? packageId)
{
    return _createToolPackageStore(toolPath).EnumeratePackages()
        // =================================^ null here. Boom.
        .Where((p) => PackageHasCommands(p) && PackageIdMatches(p, packageId))
        .OrderBy(p => p.Id)
        .ToArray();
}

// ToolInstallGlobalOrToolPathCommand.cs
public ToolInstallGlobalOrToolPathCommand(
    // ...
    IToolPackageStoreQuery store = null)
    : base(parseResult)
{
    // ...
    _store = store; // `_store` assigned here. As we will see this is always null.
    // ...
}

public override int Execute()
{
    // ...
    var toolListCommand = new ToolListGlobalOrToolPathCommand(
        _parseResult
        // Argument for `_createToolPackageStore` here. `_createToolPackageStore()` is always null.
        , toolPath => { return _store; }
        );
    // ...
}

// ToolUpdateGlobalOrToolPathCommand.cs
public ToolUpdateGlobalOrToolPathCommand(ParseResult parseResult,
    CreateToolPackageStoresAndDownloaderAndUninstaller createToolPackageStoreDownloaderUninstaller = null,
    CreateShellShimRepository createShellShimRepository = null,
    IReporter reporter = null,
    IToolPackageStoreQuery _store = null) // < 5th parameter. This is never explicitly passed in and will always be default value.
    : base(parseResult)
{
    // ...
    _toolInstallGlobalOrToolPathCommand = new ToolInstallGlobalOrToolPathCommand(
            parseResult,
            packageId,
            _createToolPackageStoreDownloaderUninstaller,
            _createShellShimRepository,
            reporter: reporter,
            store: _store); // Argument for `ToolInstallGlobalOrToolPathCommand._store` here. This is always null.
}

// ToolUpdateCommand.cs
public ToolUpdateCommand(
    // ...
    )
    : base(result)
{
    // ...
    _toolUpdateGlobalOrToolPathCommand =
        toolUpdateGlobalOrToolPathCommand
        ?? new ToolUpdateGlobalOrToolPathCommand(
            result,
            createToolPackageStoreDownloaderUninstaller,
            createShellShimRepository,
            reporter); // < Only 4 arguments. `_store` will use default value `null`.
    // ...
}

Proposed Fix

// ToolUpdateCommand.cs
  public ToolUpdateCommand(
      // ...
      )
      : base(result)
  {
      // ...
      _toolUpdateGlobalOrToolPathCommand =
          toolUpdateGlobalOrToolPathCommand
          ?? new ToolUpdateGlobalOrToolPathCommand(
              result,
              createToolPackageStoreDownloaderUninstaller,
              createShellShimRepository,
-             reporter);
+             reporter,
+             ToolPackageFactory.CreateToolPackageStoreQuery());
      // ...
  }

I'm not sure if this would solve all problems though. I tested it (by first adding the public NuGet gallery to NuGet.config because the built-in overrides do not have the tools I want to test with) and it no longer throws NRE. Will try to run the tests to catch any regressions.

Why Not Discovered in Test?

There are existing test for the tool update -g --all scenario, so why didn't the test catch the problem?

It is because the test used a mocked store that masked the problematic code path.

dotnet / sdk