Skip to content

[Robustness] ProcessLauncher.Start — synchronous launch with TimeoutMs == 0 falls into unbounded WaitForExit() and can hang the service #952

@Christophe-Rogiers

Description

@Christophe-Rogiers

Severity: Warning / Robustness

File: src/Servy.Service/ProcessManagement/ProcessLauncher.cs:93-101

            // Synchronous mode: Wait for exit while pulsing the SCM
            if (options.TimeoutMs > 0)
            {
                WaitForExitWithHeartbeat(process, options, logger); // pass null-safe heartbeat
            }
            else
            {
                process.UnderlyingProcess.WaitForExit();
            }

Explanation: The Start method has two execution modes — fire-and-forget (returns at line 67) and synchronous wait. The fire-and-forget path is gated by options.FireAndForget and short-circuits before this block. So the only way to reach the else branch above is:

  • FireAndForget == false (we are in synchronous mode)
  • AND TimeoutMs <= 0 (no timeout configured)

In that combination the launcher calls WaitForExit() with no timeout and no SCM heartbeat. That has two failure modes inside a Windows Service:

  1. Indefinite hang — if the child never exits (which is the normal state of any long-running pre-launch hook configured by a user who intended fire-and-forget but forgot FireAndForget = true), OnStart blocks forever and the SCM eventually kills the host with error 1053.
  2. No SCM heartbeat — even if the child eventually exits, the OnScmHeartbeat pulse is never invoked. Anything taking longer than the SCM start timeout (default ~30s) crashes the service with "did not respond in a timely fashion".

Compare this to the synchronous path, which both honors a timeout AND pulses SCM:

private static void WaitForExitWithHeartbeat(IProcessWrapper process, ProcessLaunchOptions options, IServyLogger logger)
{
    var sw = Stopwatch.StartNew();

    while (!process.WaitForExit(options.WaitChunkMs))
    {
        options.OnScmHeartbeat?.Invoke(options.ScmAdditionalTimeMs);
        ...
    }
}

Today the only callers I can see (Service.cs pre-launch / post-launch / failure program paths) all set either FireAndForget = true or a clamped TimeoutMs, so this branch is dormant. But it's a foot-gun: any future caller who instantiates ProcessLaunchOptions with the default TimeoutMs = 0 and forgets to set FireAndForget = true will deadlock the host service silently.

Suggested fix — make the foot-gun unreachable:

Either reject the combination explicitly:

if (options.TimeoutMs <= 0)
{
    throw new ArgumentException(
        "Synchronous launch requires TimeoutMs > 0. Set FireAndForget = true for unbounded launches.",
        nameof(options));
}
WaitForExitWithHeartbeat(process, options, logger);

Or, if a 'wait without timeout' is genuinely a use case, at least pulse the SCM in the wait loop:

while (!process.WaitForExit(options.WaitChunkMs))
{
    options.OnScmHeartbeat?.Invoke(options.ScmAdditionalTimeMs);
}

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