镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Improve the performance of Process.HandleCount on Linux - #136630

Open
martincostello wants to merge 3 commits into
dotnet:mainfrom
martincostello:process-linux-handlecount-and-lazy-threads
Open

martincostello wants to merge 3 commits into
dotnet:mainfrom
martincostello:process-linux-handlecount-and-lazy-threads

Conversation

@martincostello

@martincostello martincostello commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Improve the performance of two things that Process does on Linux, which are most noticeable to applications that read process information periodically, such as when collecting process metrics. Each change is in its own commit.

  1. Count file descriptors without a stat for each of them (Process.HandleCount)

    HandleCount was implemented with Directory.GetFiles("/proc/<pid>/fd"). Every entry of that directory is a symlink, and FileSystemEntry.Initialize() eagerly stats every DT_LNK entry to find out if it refers to a directory, before any filtering happens, so the cost grew with the number of descriptors (about 8 ms and 290 KB per call with 3,000 descriptors). The entries are now counted using Interop.Sys.OpenDir()/ReadDir()/CloseDir(), which don't stat anything, skipping . and .. and, for the current process, the descriptor that the enumeration itself opens.

Part 2 moved to #137497.

  1. Defer reading the threads of a process until they are used (ProcessInfo)

    Creating the ProcessInfo for a process (which Refresh() followed by any property such as WorkingSet64, VirtualMemorySize64 or HandleCount does, as does Process.GetProcesses() for every process) read /proc/<pid>/task/<tid>/stat for every thread of the process, which costs about 8 µs and 1.4 KB per thread, even if Threads is never used. The thread information is now read on the first access to Process.Threads. The original ProcPid is stored on the ProcessInfo, so processes that were resolved through the /proc/self mapping (such as in a different PID namespace) still resolve to the correct thread directory, and the read is skipped if the process is known to have exited, so the threads of a different process that has since reused the PID are never returned, in the same way as for HandleCount.

This was found when profiling an ASP.NET Core application that collects process metrics using OpenTelemetry, where Process.HandleCount (Directory.GetFiles() to FileStatus.RefreshCaches() to stat()) was the most expensive part of each metrics collection, at about 0.6% of the CPU time of the process under load.

Behavior changes

  1. HandleCount now counts descriptors that refer to directories, whereas Directory.GetFiles() excluded them as a side effect (a process with one directory descriptor now has a count that is 1 higher, in line with /proc/<pid>/fd and with Windows counting every handle).
  2. If /proc/<pid>/fd cannot be opened due to permissions (such as for another user's process), HandleCount still throws UnauthorizedAccessException, but the message is now the operating system's text (Permission denied) rather than Access to the path '/proc/1/fd' is denied..
  3. Process.Threads is now read when first used rather than when the information for the process is created, so it can reflect a slightly later point in time than the other properties from the same Refresh(). If the process has exited in the meantime, the list is empty, rather than the threads of the earlier snapshot.

Benchmarks

The benchmarks measure the time and allocations of a call (Refresh() and then reading the property), as called by a process that has the given number of extra, idle threads and open file descriptors (about 40% regular files, 40% TCP sockets and 20% anonymous pipes), including the threads and descriptors that the runtime itself has. The "OpenTelemetry-style collection" does Refresh(), then WorkingSet64, VirtualMemorySize64, Threads.Count and HandleCount, as OpenTelemetry.Instrumentation.Process does. Refresh() and Threads.Count are included to show that code that does need the threads is not made slower.

The baseline is main at a69255b, the commit this branch is based on. The System.Diagnostics.Process sources of main and of the branch were each compiled into the same benchmark program (see below), and run in alternating order, 6 runs of each program for each combination of threads and descriptors, in WSL2. Each cell is the median of the medians of the runs, and the allocations are exact. Ratios are branch / main, so lower is better, and reductions in allocations are in bold.

Timings are noisy, as the machine was in use, and the minimum and maximum of the repetitions within a run regularly differ by 30% or more, so differences of less than about 20% in time should be treated as noise, such as for Threads.Count.

Windows 11 host, WSL2 Ubuntu 24.04.2 LTS (kernel 6.18.54.1-microsoft-standard-WSL2)
13th Gen Intel Core i7-13700H 2.90GHz, 20 logical cores, 15.9 GB
.NET SDK 10.0.113, .NET 10.0.13 (commit 4099f83977), Release, tiered compilation
Method Threads Descriptors Median (main) Median (branch) Ratio Allocated (main) Allocated (branch) Ratio
Refresh() + HandleCount 11 76 276.3 μs 26.0 μs 0.09 25.6 KB 4,384 B 0.17
Refresh() + HandleCount 11 460 1.11 ms 80.4 μs 0.07 55.6 KB 4,380 B 0.08
Refresh() + HandleCount 11 3004 7.89 ms 496.5 μs 0.06 286.4 KB 4,384 B 0.01
Refresh() + HandleCount 51 76 421.3 μs 28.1 μs 0.07 69.1 KB 4,384 B 0.06
Refresh() + HandleCount 51 460 1.38 ms 74.6 μs 0.05 99.0 KB 4,380 B 0.04
Refresh() + HandleCount 51 3004 8.72 ms 671.0 μs 0.08 329.8 KB 4,381 B 0.01
Refresh() + WorkingSet64 11 76 67.1 μs 12.9 μs 0.19 18.0 KB 4,384 B 0.24
Refresh() + WorkingSet64 11 460 64.4 μs 13.8 μs 0.21 17.9 KB 4,380 B 0.24
Refresh() + WorkingSet64 11 3004 68.3 μs 13.1 μs 0.19 18.0 KB 4,384 B 0.24
Refresh() + WorkingSet64 51 76 231.8 μs 13.4 μs 0.06 61.6 KB 4,384 B 0.07
Refresh() + WorkingSet64 51 460 244.5 μs 13.0 μs 0.05 61.4 KB 4,380 B 0.07
Refresh() + WorkingSet64 51 3004 241.6 μs 15.2 μs 0.06 61.4 KB 4,384 B 0.07
Refresh() + Threads.Count 11 76 66.5 μs 66.6 μs 1.00 19.1 KB 19.2 KB 1.00
Refresh() + Threads.Count 11 460 67.0 μs 67.1 μs 1.00 19.0 KB 19.1 KB 1.00
Refresh() + Threads.Count 11 3004 67.5 μs 66.0 μs 0.98 19.2 KB 19.2 KB 1.00
Refresh() + Threads.Count 51 76 230.6 μs 234.4 μs 1.02 65.8 KB 65.7 KB 1.00
Refresh() + Threads.Count 51 460 236.7 μs 235.7 μs 1.00 65.6 KB 65.5 KB 1.00
Refresh() + Threads.Count 51 3004 272.5 μs 242.9 μs 0.89 65.7 KB 65.6 KB 1.00
OpenTelemetry-style collection 11 76 267.2 μs 77.9 μs 0.29 27.0 KB 19.2 KB 0.71
OpenTelemetry-style collection 11 460 1.12 ms 133.9 μs 0.12 56.9 KB 19.1 KB 0.34
OpenTelemetry-style collection 11 3004 7.35 ms 548.2 μs 0.07 287.8 KB 19.2 KB 0.07
OpenTelemetry-style collection 51 76 414.2 μs 245.1 μs 0.59 73.6 KB 65.7 KB 0.89
OpenTelemetry-style collection 51 460 1.27 ms 319.3 μs 0.25 103.5 KB 65.5 KB 0.63
OpenTelemetry-style collection 51 3004 8.84 ms 806.1 μs 0.09 334.3 KB 65.6 KB 0.20
Process.GetProcesses() and Dispose() 11 76 761.1 μs 364.2 μs 0.48 182.3 KB 111.5 KB 0.61

The HandleCount commit accounts for most of the improvement of the HandleCount rows, which grows with the number of descriptors (at about 3,000 descriptors a call goes from about 8 ms to 0.5-0.7 ms), and the deferred thread reads account for the WorkingSet64 rows and GetProcesses(), as well as for the HandleCount rows with few descriptors and many threads.

Tests

Four tests are added to ProcessTests.Unix.cs: HandleCount_OtherProcess_MatchesFileDescriptorDirectory (a child process, with a readiness handshake, compared to /proc/<pid>/fd), HandleCount_CurrentProcess_IncreasesWhenFilesAreOpened (in a RemoteExecutor child process so that other tests cannot change the descriptors) and two tests that Threads is available after other information about the current and another process was read. System.Diagnostics.Process.Tests has not been run: the shared framework and test infrastructure could not be built for Linux in the environment used, so these tests have only been checked for syntax, and CI will be the first time they run. In their place, the following were run against the same sources compiled into a harness in WSL2, for main and for each branch (55 of 55 checks passing): the handle count of another process and of the current process against /proc/<pid>/fd (including after opening and closing files), Threads after other properties and after Refresh(), GetProcesses() across all visible processes, and a zombie process. The cases for a different PID namespace (Environment.ProcessId different to /proc/self) and for a recycled PID were reproduced in the harness with unshare and ns_last_pid and behave correctly.

Benchmark code

The program is compiled with the Compile items for the System.Diagnostics.Process sources of whichever checkout is being measured (the baseline and the branch use the same program). It runs on the .NET 10 runtime, so ShimApi.cs stands in for a few newer BCL APIs used by main that are only reached by the process-start code that the benchmark does not use, and SR.Generated.cs is a stub of the resource-string accessors that the library's sources use. Run it as dotnet run -c Release -- <name> <threads> <descriptors> <output file>.

procbench.csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net10.0</TargetFramework>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <Optimize>true</Optimize>
    <EnableDefaultCompileItems>false</EnableDefaultCompileItems>
    <InvariantGlobalization>true</InvariantGlobalization>
    <DefineConstants>$(DefineConstants);TARGET_LINUX;TARGET_UNIX</DefineConstants>
    <!-- Warnings from compiling the library sources outside of the repo's build are suppressed -->
    <NoWarn>$(NoWarn);CS0436;CS8600;CS8601;CS8602;CS8603;CS8604;CS8618;CS8625;CS8765;CS8767;CS0618;CS0219;CS0169;CS0414;CS0649;CS1591;SYSLIB1054;CA1416;CA2101;SYSLIB5004;CS8981</NoWarn>
    <!-- The folder containing a checkout of the source tree to benchmark, i.e. main or the branch -->
    <SourceRoot Condition="'$(SourceRoot)' == ''">../runtime</SourceRoot>
  </PropertyGroup>
  <ItemGroup>
    <Compile Include="Program.cs" />
    <Compile Include="ShimApi.cs" />
    <Compile Include="SR.Generated.cs" />
    <!-- Plus a <Compile Include="$(SourceRoot)/src/libraries/.../*.cs" /> item for each of the files that MSBuild resolves for
         System.Diagnostics.Process.csproj with net11.0-linux (95 for the branch: 42 from System.Diagnostics.Process/src, including the new
         ProcessInfo.Linux.cs, 52 from src/libraries/Common and 1 from System.Private.CoreLib/src). -->
  </ItemGroup>
</Project>

ShimApi.cs

using System.Runtime.InteropServices;
using Microsoft.Win32.SafeHandles;

// Stand-ins for BCL APIs that exist in the .NET version the Process sources in dotnet/runtime main target,
// but not in the .NET 10 runtime used to run this benchmark. They are only used on process start code paths
// that the benchmark does not exercise.
internal static class ShimApi
{
    public const PosixSignal SIGKILL = (PosixSignal)(-9);

    public static SafeFileHandle OpenNullHandle() => File.OpenHandle("/dev/null", FileMode.Open, FileAccess.ReadWrite);

    public static SafeFileHandle OpenStdin() => new SafeFileHandle((IntPtr)0, ownsHandle: false);

    public static SafeFileHandle OpenStdout() => new SafeFileHandle((IntPtr)1, ownsHandle: false);

    public static SafeFileHandle OpenStderr() => new SafeFileHandle((IntPtr)2, ownsHandle: false);

    public static void CreateAnonymousPipe(out SafeFileHandle read, out SafeFileHandle write, bool asyncRead = false)
        => throw new PlatformNotSupportedException();
}

Program.cs

using System.Diagnostics;
using System.IO.Pipes;
using System.Net.Sockets;
using System.Text;

internal static class Program
{
    private static readonly List<object> s_keepAlive = new();
    private static readonly ManualResetEventSlim s_stop = new(false);

    private static int Main(string[] args)
    {
        // args: <variant> <threads> <fds> [json-output-file]
        string variant = args[0];
        int targetThreads = int.Parse(args[1]);
        int targetFds = int.Parse(args[2]);
        string? output = args.Length > 3 ? args[3] : null;

        EnsureThreads(targetThreads);
        EnsureFds(targetFds);

        int actualThreads = ReadStatusInt("Threads:");
        int actualFds = Directory.GetFileSystemEntries("/proc/self/fd").Length;

        using Process p = Process.GetCurrentProcess();

        var results = new List<(string Name, double Ns, double Bytes, double MinNs, double MaxNs)>();

        void Bench(string name, Action action, int samples = 7, int sampleMs = 250)
        {
            var r = Measure(action, samples, sampleMs);
            results.Add((name, r.Ns, r.Bytes, r.MinNs, r.MaxNs));
            Console.Error.WriteLine($"[{variant} t={actualThreads} fd={actualFds}] {name,-34} {r.Ns / 1000.0,10:F1} us  {r.Bytes,10:F0} B");
        }

        p.Refresh();
        int handleCount = p.HandleCount;
        int filesOnly = Directory.GetFiles("/proc/self/fd").Length;
        int allEntries = Directory.GetFileSystemEntries("/proc/self/fd").Length;
        Console.Error.WriteLine($"[{variant}] Process type from '{typeof(Process).Assembly.GetName().Name}'; HandleCount={handleCount}; Directory.GetFiles={filesOnly}; Directory.GetFileSystemEntries={allEntries} (each includes the enumerator's own descriptor)");

        Bench("HandleCount (Refresh+HandleCount)", () => { p.Refresh(); _ = p.HandleCount; });
        Bench("Refresh+WorkingSet64", () => { p.Refresh(); _ = p.WorkingSet64; });
        Bench("Refresh+Threads.Count", () => { p.Refresh(); _ = p.Threads.Count; });
        Bench("OTel-style collection", () =>
        {
            p.Refresh();
            _ = p.WorkingSet64;
            _ = p.VirtualMemorySize64;
            _ = p.Threads.Count;
            _ = p.HandleCount;
        });
        Bench("OTel-style without Threads", () =>
        {
            p.Refresh();
            _ = p.WorkingSet64;
            _ = p.VirtualMemorySize64;
            _ = p.HandleCount;
        });

        int processCount = 0;
        Bench("Process.GetProcesses()", () =>
        {
            Process[] all = Process.GetProcesses();
            processCount = all.Length;
            foreach (Process x in all)
            {
                x.Dispose();
            }
        }, samples: 5, sampleMs: 300);

        if (output is not null)
        {
            var sb = new StringBuilder();
            sb.Append($"{{\"variant\":\"{variant}\",\"threads\":{actualThreads},\"fds\":{actualFds},\"processes\":{processCount},\"handleCount\":{handleCount},\"results\":[");
            sb.Append(string.Join(",", results.Select(r => $"{{\"name\":\"{r.Name}\",\"ns\":{r.Ns:F1},\"bytes\":{r.Bytes:F1},\"minNs\":{r.MinNs:F1},\"maxNs\":{r.MaxNs:F1}}}")));
            sb.Append("]}");
            File.WriteAllText(output, sb.ToString());
        }

        s_stop.Set();
        return 0;
    }

    private static (double Ns, double Bytes, double MinNs, double MaxNs) Measure(Action action, int samples, int sampleMs)
    {
        // Warm up so the code is tiered up.
        var sw = Stopwatch.StartNew();
        int warm = 0;
        while (sw.ElapsedMilliseconds < 1000 || warm < 50)
        {
            action();
            warm++;
        }

        // Calibrate the number of calls per sample.
        sw.Restart();
        int cal = 0;
        while (sw.ElapsedMilliseconds < 100 || cal < 3)
        {
            action();
            cal++;
        }

        int perSample = Math.Max(3, (int)(cal * (sampleMs / (double)sw.ElapsedMilliseconds)));

        var ns = new List<double>();
        var bytes = new List<double>();
        for (int s = 0; s < samples; s++)
        {
            GC.Collect();
            long a0 = GC.GetAllocatedBytesForCurrentThread();
            long t0 = Stopwatch.GetTimestamp();
            for (int i = 0; i < perSample; i++)
            {
                action();
            }

            long t1 = Stopwatch.GetTimestamp();
            long a1 = GC.GetAllocatedBytesForCurrentThread();
            ns.Add((t1 - t0) * 1e9 / Stopwatch.Frequency / perSample);
            bytes.Add((a1 - a0) / (double)perSample);
        }

        ns.Sort();
        bytes.Sort();
        return (ns[ns.Count / 2], bytes[bytes.Count / 2], ns[0], ns[^1]);
    }

    private static void EnsureThreads(int target)
    {
        while (ReadStatusInt("Threads:") < target)
        {
            var t = new Thread(() => s_stop.Wait()) { IsBackground = true };
            t.Start();
            s_keepAlive.Add(t);
            Thread.Sleep(2);
        }
    }

    private static void EnsureFds(int target)
    {
        // Open a mix of regular files, sockets and pipes (about 40%, 40% and 20% of the descriptors respectively).
        int i = 0;
        while (Directory.GetFileSystemEntries("/proc/self/fd").Length < target)
        {
            for (int j = 0; j < 20; j++)
            {
                switch (i++ % 5)
                {
                    case 0:
                    case 1:
                        s_keepAlive.Add(File.OpenHandle("/etc/hostname"));
                        break;
                    case 2:
                    case 3:
                        s_keepAlive.Add(new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp));
                        break;
                    default:
                        s_keepAlive.Add(new AnonymousPipeServerStream(PipeDirection.Out, HandleInheritability.None));
                        break;
                }
            }
        }
    }

    private static int ReadStatusInt(string key)
    {
        foreach (string line in File.ReadLines("/proc/self/status"))
        {
            if (line.StartsWith(key, StringComparison.Ordinal))
            {
                return int.Parse(line.AsSpan(key.Length).Trim());
            }
        }

        return -1;
    }
}

Note

This pull request description and the changes were generated with AI assistance (Claude Code).

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Oct 29, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Directory-enumeration errors can silently produce cached partial handle counts, while the new tests leave descriptor accounting and delayed thread access after process exit unprotected.

3 open issues

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The change breaks IOException compatibility for EMFILE failures and adds WorkingSet64 assertions that can fail immediately after child startup on Linux 6.2+.

2 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Preserve IOException mapping when opening the descriptor directory fails

src/​libraries/​System.Diagnostics.Process/​src/​System/​Diagnostics/​Process.Linux.cs:181

When opening the descriptor directory fails with EMFILE (too many open files), this fallback throws Win32Exception, whereas Directory.GetFiles previously mapped that error to IOException. Callers catching IOException would no longer handle the same failure; preserve the IOException mapping for these I/O errors.

2 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Active findings identify an exception-type compatibility regression during descriptor exhaustion and an incorrect empty thread list for a running process when procfs and PID namespaces differ.

1 open issue
2 previously missed issues

In code that hasn't changed since last review

Medium severity Preserve IOException mapping for proc directory I/O failures

src/​libraries/​System.Diagnostics.Process/​src/​System/​Diagnostics/​Process.Linux.cs:181

With cached process information, descriptor exhaustion can make opening /proc/self/fd fail with EMFILE, even though the old Directory.Exists check would succeed. Previously Directory.GetFiles translated this failure to IOException; throwing Win32Exception instead breaks callers that catch IOException to recover. Preserve the IOException mapping for these directory I/O failures.

Medium severity Skip PID-based exit checks for the current process

src/​libraries/​System.Diagnostics.Process/​src/​System/​Diagnostics/​Process.Linux.cs:192

When procfs and PID namespaces differ, GetProcesses() retains ProcPid.Self but constructs the current process's wrapper with its procfs PID. Reading HasExited first can cache an exit for that unmapped PID, causing this guard to return an empty thread list for the still-running process. Bypass the PID-based exit check for ProcPid.Self, and cover GetProcesses() followed by HasExited and Threads in the mismatched-namespace test.

1 resolved since last review

Review effort: Balanced

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@martincostello big thanks for your contribution! Could you please divide this PR into two separate ones (one for HandleCount and another for threads)? It will make it easier for me to review and merge them, especially since one of them might bring some behavior difference.

Process.HandleCount on Linux used Directory.GetFiles on /proc/<pid>/fd.
Every entry in that directory is a symlink, so the file system enumerator
stat'd the target of every file descriptor purely to filter out
directories, and allocated a string for each one.

Count the entries directly using readdir (excluding "." and ".." and, for
the current process, the descriptor of the directory stream itself). The
return value of readdir is checked so that a failed enumeration is not
reported as a partial count.

Behavior changes:
- File descriptors that refer to directories are now counted (as handles
  are on Windows). They were previously excluded as a side effect of using
  GetFiles.
- If /proc/<pid>/fd cannot be opened or read because access is denied an
  UnauthorizedAccessException is still thrown, but its message is the
  operating system's error message rather than one that includes the path.
  Other failures (except the process having exited) throw a Win32Exception.

Assisted-by: Claude Code <noreply@anthropic.com>
@martincostello
martincostello force-pushed the process-linux-handlecount-and-lazy-threads branch from fa8e9c4 to 6e82b80 Compare November 13, 2026 13:01
@martincostello martincostello changed the title Improve the performance of Process.HandleCount and refreshing a Process on Linux Improve the performance of Process.HandleCount on Linux Nov 13, 2026

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall it looks very good, I found only few nits. Thank you @martincostello !

// Hold a directory descriptor open, which a count that filtered out directories would miss.
// The directory stream is opened by the first call to MoveNext() and kept open until disposed.
using IEnumerator<string> directory = Directory.EnumerateFileSystemEntries(Path.GetTempPath()).GetEnumerator();
Assert.True(directory.MoveNext());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In theory, the TEMP can be empty.

Suggested change
Assert.True(directory.MoveNext());
_ = directory.MoveNext();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@martincostello this remained unresolved, please let me know if it's on purpose

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'd skipped it based on this feedback from Claude Code which seemed reasonable to me:

If the directory is empty, the enumerator finishes and closes its handle, so no directory descriptor is held and the test would silently stop covering that case. I switched to AppContext.BaseDirectory, which always has files, and kept Assert.True(directory.MoveNext()).

- Throw `IOException` instead of `Win32Exception`.
- Use `File.OpenNullHandle()`.
- Avoid creating a temp file.
- Compare file count using the old implementation.

@adamsitnik adamsitnik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@martincostello PTAL at my comments, thanks!


// Enumerating /proc/self/fd opens one additional descriptor (the directory stream) that is listed
// in its own results, so it is subtracted here. HandleCount excludes the descriptor it opens itself.
int expected = Directory.GetFileSystemEntries("/proc/self/fd").Length - 1;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we use Directory.GetFiles("/proc/self/fd", "*", SearchOption.TopDirectoryOnly) here as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added a comment.

// Hold a directory descriptor open, which a count that filtered out directories would miss.
// The directory stream is opened by the first call to MoveNext() and kept open until disposed.
using IEnumerator<string> directory = Directory.EnumerateFileSystemEntries(Path.GetTempPath()).GetEnumerator();
Assert.True(directory.MoveNext());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@martincostello this remained unresolved, please let me know if it's on purpose

Add a comment explaining why the test is written as-is.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Process community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants