Skip to content

[Robustness] Service.cs — Fire-and-forget PRESHUTDOWN registration races OnStart failure #758

@Christophe-Rogiers

Description

@Christophe-Rogiers

Severity: Warning
File: src/Servy.Service/Service.cs lines 525-555

Description:

The final step of OnStart schedules a fire-and-forget task to register SERVICE_ACCEPT_PRESHUTDOWN after a 500 ms delay:

if (_serviceHandle != IntPtr.Zero)
{
    _ = Task.Run(async () =>
    {
        await Task.Delay(500);

        SERVICE_STATUS status = new SERVICE_STATUS
        {
            dwServiceType = SERVICE_WIN32_OWN_PROCESS,
            dwCurrentState = SERVICE_RUNNING,
            dwControlsAccepted = SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_PRESHUTDOWN,
            ...
        };

        if (!SetServiceStatus(_serviceHandle, ref status))
        {
            int error = Marshal.GetLastWin32Error();
            _logger?.Error($\"Failed to register PRESHUTDOWN support via native Win32. Error: {error}\");
        }
        ...
    });
}

Three problems:

  1. Use-after-dispose race. If OnStart takes the failure branch — Stop() at L514 (pre-launch failed) or the outer catch (Exception ex) at L557 calling Stop() — the scheduled task will still fire ~500 ms later and invoke SetServiceStatus(_serviceHandle, ref status) against a service that has already transitioned to STOPPED and may have closed or invalidated _serviceHandle. The task has no knowledge of the stop.

  2. Unobserved task exceptions. There is no try/catch around the outer async lambda. Any exception outside the single SetServiceStatus check (e.g. an AccessViolation from the stale handle above, or an exception from the logger) becomes an unobserved task exception. TaskScheduler.UnobservedTaskException is not wired in Service startup, so the process may terminate on finalization under older runtime policies.

  3. No cancellation token. The task is not tied to _stopCts or any lifetime token, so it cannot be cancelled if Stop races.

Suggested fix:

  • Check stop state before SetServiceStatus (if (_stopCts.IsCancellationRequested || _serviceHandle == IntPtr.Zero) return;).
  • Wrap the outer lambda in try/catch and log the exception.
  • Pass a cancellation token to Task.Delay and to the whole flow so shutdown cancels the pending registration.
  • Better: register PRESHUTDOWN synchronously before ServiceBase.OnStart returns SERVICE_RUNNING, using one of the known workarounds (e.g. setting AcceptedCommands via reflection on ServiceBase), which removes the need for the 500 ms delay entirely.

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