Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Process.GetProcessesByName Memory Leak, Not Disposing Filtered Process Objects #10143

Closed
slaingod opened this issue Aug 16, 2018 · 0 comments · Fixed by #10156

Comments

@slaingod
Copy link

@slaingod slaingod commented Aug 16, 2018

There is a pretty serious memory leak in Process.GetProcessesByName around this line of code:
https://github.com/mono/mono/blob/master/mcs/class/System/System.Diagnostics/Process.cs#L512

The code creates Process objects for every process, then filters them based on the supplied processName. It does not however take the crucial step of calling Process.Dispose before doing so, on the filtered processes. Process semantics require that Dispose be called or a leak will occur.

.net Framework does not suffer from this issue, and looking at the corefx project, they use a completely different mechanism as well, only creating process objects for process names that match, rather than filtering from a full list of Process objects.

The solution is to add an else clause that disposes of processes[i]:

if (String.Compare (processName, processes[i].ProcessName, true) == 0)
    processes [size++] = processes[i];
else 
    processes[i].Dispose();

Steps to Reproduce

  1. Use Process.GetProcessByName("dummy") in a loop, disposing of any returned processes.
using System;
using System.Diagnostics;
using System.Threading;

namespace Demo
{
    class MainClass
    {
        public static void Main(string[] args)
        {
            while(true)
            {
                Process[] processes = Process.GetProcessesByName("dummy");
                {
                    for(int i = 0; i < processes.Length; i++)
                    {
                        // User must also dispose of any matched Processes that are returned
                        processes[i].Dispose();
                    }
                    Thread.Sleep(1000);
                }
            }
        }
    }
}
  1. Run mono with -profile-log:heapshot=1gc for a minute
  2. Examine the heap after a short while. You will see that System.Diagnostics.Process count increases by ~the number of running processes for each loop that has run.
mprof-report --traces --verbose output.mlpd | less

        Heap shot 2 at 136.018 secs: size: 11150128, object count: 79255, class 
count: 57, roots: 462
             Bytes      Count  Average Class name
           8601424      26224      327 System.Diagnostics.Process (bytes: +28755
28, count: +8767)
...
       Heap shot 3 at 182.534 secs: size: 14855944, object count: 105519, class count: 57, roots: 461
             Bytes      Count  Average Class name
          11464912      34954      328 System.Diagnostics.Process (bytes: +2863488, count: +8730)

Current Behavior

Process objects leak like crazy. If you have 100 processes, and only 3 match the processName, then 97 Process objects will leak.

Expected Behavior

Process objects should not leak.

On which platforms did you notice this

[ ] macOS
[X ] Linux
[ ] Windows

I expect it affects all platforms however, based on my read of the code.

Version Used:

Mono JIT compiler version 5.10.1.4 (tarball Wed Mar 21 17:14:24 UTC 2018)->

jaykrell added a commit to jaykrell/mono that referenced this issue Aug 16, 2018
Dispose rejected ones.

This mono#10143 and fix is from there.
All null the array element. Does it matter/help?
The array gets resized shortly after.
jaykrell added a commit to jaykrell/mono that referenced this issue Aug 16, 2018
Dispose rejected ones.

This mono#10143 and fix is from there.
Also null the array element. Does it matter/help?
The array gets resized shortly after.
jaykrell added a commit to jaykrell/mono that referenced this issue Aug 16, 2018
Dispose rejected ones.

This mono#10143 and fix is from there.
Also null the array element. Does it matter/help?
The array gets resized shortly after.
jaykrell added a commit to jaykrell/mono that referenced this issue Aug 16, 2018
Dispose rejected ones.

This mono#10143 and fix is from there.

I considered nulling the array elements as I go, and then the tail,
to help the GC, but does it help actually? I suspect not.
jaykrell added a commit that referenced this issue Aug 17, 2018
Dispose rejected ones.
#10143
monojenkins added a commit to monojenkins/mono that referenced this issue Aug 20, 2018
This mono#10143 and fix is from there.

I considered nulling the array elements as I go, and then the tail,
to help the GC, but does it help actually? I suspect not.
monojenkins added a commit to monojenkins/mono that referenced this issue Aug 20, 2018
This mono#10143 and fix is from there.

I considered nulling the array elements as I go, and then the tail,
to help the GC, but does it help actually? I suspect not.
akoeplinger added a commit that referenced this issue Aug 20, 2018
This #10143 and fix is from there.

I considered nulling the array elements as I go, and then the tail,
to help the GC, but does it help actually? I suspect not.
akoeplinger added a commit that referenced this issue Aug 20, 2018
This #10143 and fix is from there.

I considered nulling the array elements as I go, and then the tail,
to help the GC, but does it help actually? I suspect not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.