Skip to content

[Robustness] MainWindow.OnClosed (Servy + Manager) — Process.GetCurrentProcess() handle never disposed #873

@Christophe-Rogiers

Description

@Christophe-Rogiers

Severity: Info

Files:

  • src/Servy/Views/MainWindow.xaml.cs, line 60
  • src/Servy.Manager/Views/MainWindow.xaml.cs, line 594

Both desktop apps reach for the current process ID at window close and discard the Process instance:

protected override void OnClosed(EventArgs e)
{
    try
    {
        var currentPID = Process.GetCurrentProcess().Id;
        ProcessKiller.KillChildren(currentPID);
    }
    ...
}

Process.GetCurrentProcess() returns a Process instance that wraps the kernel's pseudo-handle and acquires a fresh kernel handle on first access of certain members. The instance implements IDisposable and Microsoft documents this exact API (along with Process.GetProcessById, Process.GetProcessesByName, etc.) as one that must be disposed.

For comparison, the rest of the codebase already does this correctly:

// ProcessKiller.cs:44
using (var current = Process.GetCurrentProcess()) { selfPid = current.Id; }

// ProcessKiller.cs:447
using (var current = Process.GetCurrentProcess()) { ... }

// ResourceHelper.cs:168
using (var process = Process.GetCurrentProcess()) { var exePath = process.MainModule?.FileName; ... }

So the same author has wrapped four call sites correctly and missed two — a quick consistency fix:

 protected override void OnClosed(EventArgs e)
 {
     try
     {
-        var currentPID = Process.GetCurrentProcess().Id;
-        ProcessKiller.KillChildren(currentPID);
+        int currentPID;
+        using (var current = Process.GetCurrentProcess())
+        {
+            currentPID = current.Id;
+        }
+        ProcessKiller.KillChildren(currentPID);
     }
     catch (Exception ex)
     {
         Logger.Error(\"Error killing child processes.\", ex);
     }

     Logger.Shutdown();
     base.OnClosed(e);
 }

Same fix in both files. Impact is small (the process is about to exit anyway, so the leaked handle dies with it) but for consistency with the four other sites it's a one-line cleanup.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions