Process pipes aren't garbage collected

[Petermarcu]

@myrup commented on Sat Dec 16 2017

Process pipes aren't garbage collected

General

When using redirection for StandardOutput (and possibly StandardError, StandardInput as well) the pipes aren't garbage collected when the Process reference is collected.

The following silly piece of code lives (probably) forever in mono, but crashes in dotnet after 217 iterations on my machine:

public static void Main() {
            for (int i = 0; i < int.MaxValue; i++) {
                Process.Start(new ProcessStartInfo {
                    FileName = "ls",
                    UseShellExecute = false,
                    RedirectStandardOutput = true
                });
                Console.WriteLine(i);
            }
}

Adding a .StandardOutput.Close() extends lifetime 5 fold, indicating other things are not being collected.

.NET Command Line Tools (2.0.2)

Product Information:
 Version:            2.0.2
 Commit SHA-1 hash:  a04b4bf512

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  10.13
 OS Platform: Darwin
 RID:         osx.10.12-x64
 Base Path:   /usr/local/share/dotnet/sdk/2.0.2/

Microsoft .NET Core Shared Framework Host

  Version  : 2.0.0
  Build    : e8b8861ac7faf042c87a5c2f9f2d04c98b69f28d

[EDIT] Add C# syntax highlighting by @karelz

[danmoseley]

@myrup what does it do on .NETFramework?

[myrup]

@danmosemsft 4.5 on windows seems consistent with mono.

[tmds]

When I run this code I get:

Unhandled Exception: System.ComponentModel.Win32Exception: Resource temporarily unavailable

This is because fork returns EAGAIN:

A system-imposed limit on the number of threads was encountered. There are a number of limits that may trigger this
error: the RLIMIT_NPROC soft resource limit (set via setrlimit(2)), which limits the number of processes and threads for a
real user ID, was reached; the kernel's system-wide limit on the number of processes and threads, /proc/sys/ker‐
nel/threads-max, was reached (see proc(5)); or the maximum number of PIDs, /proc/sys/kernel/pid_max, was reached (see
proc(5)).

The system is keeping these processes until someone calls waitpid and no-one is doing that.

Like mono, we should use SIGCHLD to be notified when a child process terminates and then free up those system resources.

[strajkovmsft]

Any updates on this? Also having this issue with process pipes not getting disposed and causing application issues in a long running application and would love to know what the status is.

[danmoseley]

@strajkovmsft I don't think anyone is working on a fix. This is a Unix issue apparently, are you on Unix?

[strajkovmsft]

Yes, Ubuntu 16.04. I don’t know that I’d call it a Unix issue, the underlying streams backing stdout and stderr never have dispose called on them, and if you try to after using async, you get an exception for mixing sync and async when trying to access the steam to dispose it. Don’t dispose the stream and you won’t free the pipe, don’t free the pipe and you leak file descriptors. Leaking file descriptors leads to EMFILE (I believe that’s the code) which manifests as too many files open. This is all intended behavior on the Linux side. Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Dan Moseley <notifications@github.com> Sent: Friday, January 5, 2018 6:44:46 PM To: dotnet/corefx Cc: Stefan Rajkovic; Mention Subject: Re: [dotnet/corefx] Process pipes aren't garbage collected (#25962) @strajkovmsft<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fstrajkovmsft&data=02%7C01%7Cstefan.rajkovic%40microsoft.com%7C4b734415364d49287ebd08d554964dec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636507926886698395&sdata=QMkqnl04tQiYjfJTsg9xsSMp2NQV65LytIv54LnzkEI%3D&reserved=0> I don't think anyone is working on a fix. This is a Unix issue apparently, are you on Unix? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdotnet%2Fcorefx%2Fissues%2F25962%23issuecomment-355695523&data=02%7C01%7Cstefan.rajkovic%40microsoft.com%7C4b734415364d49287ebd08d554964dec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636507926886698395&sdata=aNSPXW1ODfsfVN07EGCKZ1PDd4gmLNRuIIklvsoW1Ag%3D&reserved=0>, or mute the thread<https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAhhiWuzYJVfiLIXnKszfWxh8b5PNVopMks5tHrPugaJpZM4REb-n&data=02%7C01%7Cstefan.rajkovic%40microsoft.com%7C4b734415364d49287ebd08d554964dec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636507926886698395&sdata=hGquxCQIZ%2FJtga5cqApv9bi8KfgRB5tKWBS%2Br8MmqEc%3D&reserved=0>.

[danmoseley]

@tmds can you point to where Mono is listening for SIGCHLD? I don't see it.

I do see a little extra in the Process class Dispose() method

In .NET/.NET Core the code comments:

                //Don't call close on the Readers and writers
                //since they might be referenced by somebody else while the 
                //process is still alive but this method called.
                _standardOutput = null;
                _standardInput = null;
                _standardError = null;

but Mono is a little cleverer and does close if they were never requested by the user
https://github.com/mono/mono/blob/master/mcs/class/referencesource/System/services/monitoring/system/diagnosticts/Process.cs#L1365

I suspect if the code above accessed the streams on the process, it would have the same behavior as .NET Core.

As for 4.5 on Windows being OK, perhaps Windows just runs out of resources less quickly in this scenario.

[myrup]

Labelling this as an enhancement baffles me a bit. I consider it a serious bug that it isn't possible to have a long running process that utilises subprocesses on *nix.

[strajkovmsft]

Also "Windows just runs out of resources less quickly" is disingenuous. Linux is not out of resources, a limit on the number of file handles a process can have open is the intended behavior. If you have root access to the machine you can raise the limit, but that simply delays the catastrophic failure. I'm working on a patch now.

[tmds]

@danmosemsft the signal handler is set here: https://github.com/mono/mono/blob/17a2fba78de10678cf1ad903d410b057340a2795/mono/metadata/w32process-unix.c#L1610

[danmoseley]

@tmds thank you, I was looking in *.cpp ...

[danmoseley]

The Mono handler seems to end up doing the waitpid loop (in mono_runtime_do_background_work) via the finalizer thread (?). I am not sure whether we do such things on the finalizer thread - we should probably use the threadpool.

Our ProcessWaitState.Unix.cs "wraps" waitpid() so that one can do WaitForExit against multiple Process objects over the same PID. It expects to be the only thing calling waitpid(). One possibility is to build on that, essentially do (untested)

foreach(int pid in pids) // pids recorded when doing fork()
{
     using (var pwsh = new ProcessWaitState.Holder(pid))
              if (pwsh.GetExited(out _))
                      // remove from list of pids
}

Would be potentially a race with the PID being reused.

[strajkovmsft]

@danmosemsft we probably just want to wait on the process that we're currently closing right? So something like this in Process.Unix.cs:

        /// <summary>Additional logic invoked when the Process is closed.</summary>
        private void CloseCore()
        {
            if (_waitStateHolder != null)
            {
                ThreadPool.QueueUserWorkItem(() =>
                {
                    WaitForExitCore(Timeout.Infinite);
                    _waitStateHolder.Dispose();
                });
            }
        }

[danmoseley]

@strajkovmsft it looks like that would kick off an async Task that would poll at least every 100ms, but the process might not exit for an arbitrary time, and there might be many of them, so many tasks. I think ideally it would only run on SIGCHLD, and it would loop over all PID's we have started, and quit.

Mono also does this at the native level, which means that if any other managed code leads to a fork(), it is covered. Right now in the BCL only Process does this so maybe it's OK that it is handled at that level.

[danmoseley]

cc @stephentoub in case he wishes to comment.