Skip to content

[Correctness] HandleHelper.cs — Silent catch swallows Kill() failure on handle.exe timeout #771

@Christophe-Rogiers

Description

@Christophe-Rogiers

Severity: Info

Location: src/Servy.Core/Helpers/HandleHelper.cs:93-97

Code:

if (!process.WaitForExit(AppConfig.HandleExeTimeoutMs))
{
    try { process.Kill(); } catch { /* Ignore cleanup errors */ }
    throw new TimeoutException($"handle.exe timed out. Stderr: {errorBuilder}");
}

Explanation:
When handle.exe times out, the helper tries to kill the runaway process but silently swallows any exception from Kill(). In practice Kill() can fail with:

  • InvalidOperationException — the process already exited (benign)
  • Win32Exception — access denied (real, leaves handle.exe alive)
  • NotSupportedException — process was not started via .NET (won't happen here, but generic catch hides it)

On access-denied failures, handle.exe remains running. Repeated invocations (e.g. KillProcessesUsingFile in a retry loop) can accumulate orphaned handle.exe processes that each hold a lock on the file the caller was trying to free — the exact opposite of the intended behavior — with zero log evidence.

Suggested fix:
Filter the catch and log at Warn:

try { process.Kill(); }
catch (InvalidOperationException) { /* Already exited, expected */ }
catch (Exception killEx)
{
    Logger.Warn($"Failed to kill handle.exe after timeout (PID {process.Id}): {killEx.Message}");
}

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