0

I've detected a certain behaviour in my code that I'm unable to explain and even though it's working fine, not fully understanding what's going on is really bothering me. The script below creates and runs n_total processes of notepad but only allows a maximum of n_cpus notepads running at each time. In the beginning, n_cpus processes are launched and the rest only begin if the one or more running notepads terminate. Each notepad process can be terminated by the user simply by closing its window which triggers the event Process.Exited in the code. Now, inside the loop, the variable p is re-used to instantiate the class Process every time a new notepad is required and every object p subscribes to the event Process.Exited by p.Exited += p_Exited; Consider that we have n_cpus = 3 and run the code until it generates those 3 simultaneous notepads. I would expect that only the last instance p would fire the event since I'm re-using p and p.Exited belongs to the object, but no... No matter what notepad I close, the event is fired and a new notepad appears. What's going on? Is there some kind of objectless list of delegates EventHandler that remembers every process I create?

using System;
using System.Diagnostics;
using System.Threading;

class Program
{
    // Summary: generates processes for "n_total" notepads allowing for a maximum of "n_cpus"
    //          notepads at each time. Every time a notepad closes another appears until all are run.
    //          A single variable "p" instantiates the class "Process" and the event "p.Exited"
    //          updates the number of running processes "n_running".

    static int n_running = 0; // number of notepads running each time
    static void Main()
    {
        int n_cpus = 3;
        int n_total = 3 * n_cpus;
        int i_run = 0;

        while (i_run < n_total) // Process generating routine until all are run
        {
            if (n_running < n_cpus) // Only a maximum of n_cpus running at each time
            {
                n_running++;
                i_run++;

                Process p = new Process(); // A new object per process
                p.StartInfo.FileName = "cmd.exe";
                p.StartInfo.Arguments = "/c notepad.exe";
                p.StartInfo.UseShellExecute = false;
                p.StartInfo.CreateNoWindow = true;
                p.EnableRaisingEvents = true;
                p.Exited += p_Exited; // Is this associated with a particular object "p", right?
                p.Start();
            }
            else Thread.Sleep(1000); // Waits 1s before checking for new terminated processes
        }
    }

    static private void p_Exited(object sender, EventArgs e)
    {
        n_running--; // Updates the number of active processes. Triggers new future processes
    }
}
epp
  • 3
  • 2
  • You create a new Process, set it to raise events and subscribe an event to notify the exit. Everything works as you asked it to. – Jimi Dec 30 '17 at 14:32
  • The usage of Cmd.exe is not correct, it only blocks when it starts a console mode process. You would have to use `cmd.exe /c start /wait notepad.exe`. But just simpler to start Notepad.exe directly without cmd.exe. And fix the other bug, `int` is not an appropriate substitute for SemaphoreSlim. – Hans Passant Dec 30 '17 at 16:09
  • @Hans Interesting that about the SemaphoreSlim class, it does seem appropriate in this case but I'll have to read some more about it. Now for the `cmd.exe` this is actually for another exe that must be launched from the cmd (or at least it's more convenient that way). The `start /wait` change that you propose however, only makes a difference when the app is launched from the command line and not from a command script. In other words it always waits for notepad to close. Saw it here under **Run a Program**: [link](https://ss64.com/nt/start.html) – epp Dec 30 '17 at 17:06
  • No, start /wait works just as well on the command line or a command script. If you need prevent cmd.exe from exiting too early then you'll have to use it. Try it. – Hans Passant Dec 30 '17 at 17:30

2 Answers2

0

The OS seems to do that for you because you enabled raising events via EnableRaisingEvents = true.

See:

Wouter
  • 2,540
  • 19
  • 31
0

I think you are possibly making a couple of faulty assumptions here:

Variables are not objects, they are references to objects. When you assign an object to a variable (via new or whatever), you are not "replacing" or otherwise removing or deleting the object that was previously referenced by that variable. If there are no more references to the previous object it might be garbage collected at some point, but the object still exists and there could be other things keeping it "alive" (by containing a reference to it). Some compiler optimizations can even cause the opposite to happen: an object can get garbage collected before the variable that references it goes out of scope if the compiler determines that the variable is not being used again.

The event pattern is probably the single largest source of "memory leaks" in C# because many developers fail to realize that when you register an event, the lifetime of the object being listened to is now expanded to the lifetime of the listener (because the object being listened to, now has a reference to the listener, this reference causes the garbage collector not to collect the object). Since your p_Exited is static, all of the Process objects you create are now "rooted" (will not be garbage collected) until you unregister the event.

the variable p is re-used to instantiate the class Process

You aren't even actually "reusing" the p variable. It is declared within the scope of the loop, so every time through the loop, p is actually a "brand new" variable. This becomes an especially important distinction when it comes to closures (even the C# language team got this wrong).

Dave M
  • 2,863
  • 1
  • 22
  • 17
  • Thank you for your explanation. If I understood correctly... Even though I can't access the info of each individual `Process p = new Process();` (because `p` now refers to other object), all those new objects will remain in memory and will fire the event when its associated process terminates. In the case of a large number of `n_total` processes, shouldn't I forcefully release all the memory associated with terminated process? Something like `(Process)sender.Close();` inside `p_Exited`? – epp Dec 30 '17 at 16:14
  • The only way you can "forcefully" release memory is by calling GC.Collect() to force the garbage collector to run, and there are extremely few situations where one should do that. What you should do is make sure that there are no remaining references to the object by unregistering the event handler: `((Process)sender).Exited -= p_Exited` – Dave M Dec 30 '17 at 17:32