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:
-
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.
-
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.
-
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.
Severity: Warning
File:
src/Servy.Service/Service.cslines 525-555Description:
The final step of
OnStartschedules a fire-and-forget task to registerSERVICE_ACCEPT_PRESHUTDOWNafter a 500 ms delay:Three problems:
Use-after-dispose race. If
OnStarttakes the failure branch —Stop()at L514 (pre-launch failed) or the outercatch (Exception ex)at L557 callingStop()— the scheduled task will still fire ~500 ms later and invokeSetServiceStatus(_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.Unobserved task exceptions. There is no try/catch around the outer async lambda. Any exception outside the single
SetServiceStatuscheck (e.g. an AccessViolation from the stale handle above, or an exception from the logger) becomes an unobserved task exception.TaskScheduler.UnobservedTaskExceptionis not wired in Service startup, so the process may terminate on finalization under older runtime policies.No cancellation token. The task is not tied to
_stopCtsor any lifetime token, so it cannot be cancelled if Stop races.Suggested fix:
if (_stopCts.IsCancellationRequested || _serviceHandle == IntPtr.Zero) return;).Task.Delayand to the whole flow so shutdown cancels the pending registration.