12

For example, see

How to get the current ProcessID?

No one bothered to call Dispose for an object returned by System.Diagnostics.Process.GetCurrentProcess(). Should it actually be called? Please explain why.

Community
  • 1
  • 1
user10101
  • 1,704
  • 2
  • 20
  • 49
  • 4
    Simple: If it implements `IDisposable`, call `Dispose`. It may or may not keep a handle to unmanaged memory or some other resource, you just don't know the current implementation, so do it. – Samuel Jan 19 '15 at 11:24
  • 3
    You don't have to call it. So you got a Yes and a No answer, you still don't know anything. Don't ask this question. You can either always dispose, you'll never be wrong. Or you spend a minute writing a little test app that does it a hundred million times. Now you know the real answer. – Hans Passant Jan 19 '15 at 11:52

2 Answers2

5

Yes, and actually it is important too. If you see the actual source, you will see the Dispose isn't just inherited from Component, it does something too.

It seems to me, looking at that code, that it is most important when EnableRaisingEvents is set to true, since that involves creating a wait handle. That handle needs to be released in order to prevent memory and handle leaking.

Patrick Hofman
  • 153,850
  • 22
  • 249
  • 325
  • There is no leak. A delayed release of a wait handle is pretty innocent, they are the cheapest OS object you could allocate. Also quite unlikely he subscribed the Exited event for his own process ;) – Hans Passant Jan 19 '15 at 11:40
  • @HansPassant: I guess the link was just a sample. When you are doing heavy processing using handles, you might run out of them. And all that even doesn't matter, you should call `Dispose` if the object implements `IDisposable`. – Patrick Hofman Jan 19 '15 at 11:42
  • 1
    That's the cargo cult approach. It doesn't really belong on a site like SO. But hard to get rid of. – Hans Passant Jan 19 '15 at 11:45
  • @HansPassant: Could you translate that to Dutch? ;) Not sure what you mean. In my opinion, if you don't know what it does, it's better to stay on the safe side and call `Dispose`. – Patrick Hofman Jan 19 '15 at 11:50
  • 6
    Dat kan. Een informatief antwoord neemt niet aan dat wat iedereen doet is automaties het korrekte antwoord. Hebt je het geprobeert? Makkelijk te doen. – Hans Passant Jan 19 '15 at 11:55
  • 1
    @HansPassant: lol. English is better. Your Dutch is a little rusty. – Patrick Hofman Jan 19 '15 at 11:57
4

That`s a tough call.

You may not have to call Dispose for the Process instance you got from the Process.GetCurrentProcess() in case you did not touch the Handle property as well as some other sensitive spots.

Lets have a look at the Process.Close method which contains the essence of Dispose logic.

    public void Close()
    {
        if (this.Associated)
        {
            if (this.haveProcessHandle)
            {
                this.StopWatchingForExit();
                this.m_processHandle.Close();
                this.m_processHandle = null;
                this.haveProcessHandle = false;
            }
            this.haveProcessId = false;
            this.isRemoteMachine = false;
            this.machineName = ".";
            this.raisedOnExited = false;
            this.standardOutput = null;
            this.standardInput = null;
            this.standardError = null;
            this.Refresh();
        }
    }

You can see that something real occurs here only if the Process instance has a process handle. Refresh method has nothing of interest for our topic.

If you look further, you will see that the process handle can be obtained (and thus held) by the Process instance when the Handle property is accessed. This is not the only case though!

    public IntPtr Handle
    {
        get
        {
            this.EnsureState(Process.State.Associated);
            return this.OpenProcessHandle().DangerousGetHandle();
        }
    }

As a generic rule: if it implements IDisposable - you ought to call Dispose.

In your specific case, if you only touch the current process name or something as innocent, you can omit the Dispose call and get away with it.

Here is an example:

        Process process = Process.GetCurrentProcess();

        var fieldInfo = typeof(Process).GetField("haveProcessHandle", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
        var v1 = fieldInfo.GetValue(process);
        //v1 is false. Explicit Dispose is not necessary.

        var processName = process.ProcessName;
        var v2 = fieldInfo.GetValue(process);
        //v2 is false. Explicit Dispose is not necessary.

        var processHandle = process.Handle;
        var v3 = fieldInfo.GetValue(process);
        //v3 is true. Bah. Explicit Dispose IS necessary from now on.

I use the reflection for one sole reason: if you monitor the process variable via Visual Studio debugger, it is going to walk through the properties and read the dreaded Handle property.

Process class is a perfect example of a "bad design pattern" as it drastically changes the object state in a get accessor.

Zverev Evgeniy
  • 3,643
  • 25
  • 42