dotnet / diagnostics

This repository contains the source code for various .NET Core runtime diagnostic tools and documents.
MIT License
1.19k stars 354 forks source link

Diagnostics client fails to create dump when path has spaces #5020

Open nohwnd opened 1 month ago

nohwnd commented 1 month ago

Description

.NET diagnostics client fails when path to dump file has spaces in it. This reproduces both on net8 and net9, and fails with error:

[createdump] The pid argument is no longer supported coming from here

It can be fixed by wrapping the path to extra "".

I am using the latest version of the client package.

Reproduction Steps

<!-- file DumpError.csproj -->
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net8.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.Diagnostics.NETCore.Client" Version="0.2.547301" />
  </ItemGroup>

</Project>
// file Program.cs
using Microsoft.Diagnostics.NETCore.Client;
using System.Diagnostics;

public static class Program
{
    public static int Main(string[] args)
    {
        if (args.Length > 0 && args[0] == "--hang")
        {
            Thread.Sleep(int.MaxValue);
        }

        var path = Path.Combine(AppContext.BaseDirectory, "DumpError.exe");
        Process? process = null;
        try
        {
            process = Process.Start(path, "--hang");
            var diagnosticClient = new DiagnosticsClient(process.Id);
            Thread.Sleep(1_000);
            diagnosticClient.WriteDump(DumpType.Full, $"{Path.Combine(AppContext.BaseDirectory, $"my dump with spaces {Stopwatch.GetTimestamp()}.dmp")}", logDumpGeneration: true);
        }
        finally
        {
            process?.Kill();
        }

        return 0;
    }
}

Expected behavior

Spaces are correctly escaped and my dump is written to disk.

Actual behavior

[createdump] The pid argument is no longer supported error is shown and dump is not written.

Regression?

No.

Known Workarounds

Add quotes around the path, or move to folder without spaces.

Configuration

Win 11, 8.0.110, but reproduces on latest net9 as well.

Other information

No response

mikem8361 commented 1 month ago

This is an issue in createdump with its command line parsing. It doesn't like spaces in the dump file.

It sounds like you have a workaround by using extra "".

nohwnd commented 1 month ago

Yes we have. Unless you think this will complicate things for use down the line? E.g. your fix adding quotes to the path, and now the path will be double quoted.

mikem8361 commented 1 month ago

Yes, that will be a problem. And I realized that leaving this issue in the diagnostics repo and fixing in Microsoft.Diagnostics.NETCore.Client might be better since it will fix it across runtime versions. I may move it back or re-open the diagnostics repo issue

nohwnd commented 1 month ago

Yes fix across runtimes is definitely what we would prefer, testing.platform / VSTest are running tests down to netcoreapp3.1 and having to figure out what the TFM of tests is and apply / not apply a fix would complicate things for us.

Thank you.

mikem8361 commented 3 weeks ago

As it turns out, this is only a problem on Windows and how the runtime on Windows builds the command line for createdump here. There is no problem on Linux/MacOS runtimes.

We haven't decided where (client code or runtime) best place to fix this yet and it isn't that high of a priority since there is a workaround.