1

I have the following piece of code which creates a process, attaches to a job object and in finally block, disposes the job object and then the process. I still encounter CA2000:DisposeObjectsBeforeLosingScope code analysis error. Any ideas how I can get rid of this Code Analysis error?

        Process process = null;
        JobObject processGroupLimitJobMemory = null;

        try
        {
            // Code to create process, 
            // initialize job object and attach process to job object

            // Code to create process and attach to job object           
            processGroupLimitJobMemory = new JobObject();
            var process = new Process();
            process.StartInfo = new ProcessStartInfo()
            {
                 WorkingDirectory = @"c:\Windows\System32",
                 FileName = "notepad.exe",
                 Arguments = null,
                 UseShellExecute = false
            };
            process.Start();
            processGroupLimitJobMemory.AttachProcess(process);
            return;
        }
        catch (InvalidOperationException e)
        {                
            return;
        }
        finally
        {
            if(processGroupLimitJobMemory != null)
            {                    
                processGroupLimitJobMemory.Dispose();
            }

            if (process != null)
            {
                try
                {
                    if (!process.HasExited)
                    {                            
                        Utilities.KillProcessTree(process);
                    }
                }
                catch (InvalidOperationException e)
                {
                    // Trace
                }
                catch (ExternalException e)
                {
                    // Trace
                }
                finally
                {
                    process.Dispose();
                }
            }                
Yam
  • 293
  • 3
  • 12
  • 1
    The code you've shown us shouldn't generate a code analysis error - because it shouldn't even compile. You've got two declarations of a local variable called `process`. – Damien_The_Unbeliever Jan 21 '15 at 07:27

2 Answers2

1

There is a code branch through the first catch (InvalidOperationException e) exception handler which doesn't dispose of processGroupLimitJobMemory. Wherever possible, I would tend to use the using synctatic sugar around IDisposables, as it simplifies nested and branched dispose scenarios for us - adjacent usings can also be stacked to avoid nesting. I believe both process and processGroupLimitJobMemory can be placed in using, and the code re-formatted as follows:

try
{
    using (var processGroupLimitJobMemory = new JobObject())
    using (var process = new Process())
    {
        try
        {
            process.StartInfo = new ProcessStartInfo
            {
                WorkingDirectory = @"c:\Windows\System32",
                FileName = "notepad.exe",
                Arguments = null,// R# indicates this is a not null attribute
                UseShellExecute = false
            };
            process.Start();
            processGroupLimitJobMemory.AttachProcess(process);
        }
        finally
        {
            if (!process.HasExited)
            {
                try
                {
                    Utilities.KillProcessTree(process);
                }
                catch (InvalidOperationException e)
                {
                    // Trace
                }
                catch (ExternalException e)
                {
                    // Trace
                }
            }
        }
    }
}
catch (InvalidOperationException e)
{
    return;
}
Community
  • 1
  • 1
StuartLC
  • 104,537
  • 17
  • 209
  • 285
  • How so? The `finally` clause should still execute even if that `catch` is entered (Or I may be missing some subtlety here - I'm not fully awake yet - but if so, it probably needs more explanation here about exactly what execution path you think is being followed where the cleanup fails) – Damien_The_Unbeliever Jan 21 '15 at 07:31
  • @Damien_The_Unbeliever you are right - It was I who wasn't fully awake. FxCop is possibly tripping up on the branches in the `finally` - e.g. if `processGroupLimitJobMemory.Dispose();` throws then the `process` Dispose will be missed. I would still recommend converting to the `using` approach. – StuartLC Jan 21 '15 at 07:45
1

Moving the dispose of the process ahead of the dispose of the JobObject in the finally worked

    finally
    {            
        if (process != null)
        {
            try
            {
                if (!process.HasExited)
                {                            
                    Utilities.KillProcessTree(process);
                }
            }
            catch (InvalidOperationException e)
            {
                // Trace
            }
            catch (ExternalException e)
            {
                // Trace
            }
            finally
            {
                process.Dispose();
            }
        }                
        if(processGroupLimitJobMemory != null)
        {                    
            processGroupLimitJobMemory.Dispose();
        }
  }
Yam
  • 293
  • 3
  • 12