Severity: Warning
File: src/Servy.Service/Service.cs lines 1531-1546
Description:
OnProcessExited is the Exited-event handler and is declared async void (required for event-handler signatures). The current body awaits _healthCheckSemaphore.WaitAsync() before the try block starts:
private async void OnProcessExited(object sender, EventArgs e)
{
if (_isTearingDown || _disposed) return;
_logger?.Warn("Child process exit detected via event.");
ClearProcessState();
bool needsRecovery = false;
bool shouldStop = false;
int exitCode = -1;
// Use WaitAsync instead of Wait to avoid blocking thread pool threads
await _healthCheckSemaphore.WaitAsync(); // ← line 1543, OUTSIDE try
try
{
try
{
exitCode = _childProcess?.ExitCode ?? -1;
}
...
#473 (closed) correctly moved this from blocking Wait() to WaitAsync(), but left the await outside the exception-handling boundary.
Failure mode: the (_isTearingDown || _disposed) check on line 1533 is not synchronized with teardown — between that check and the await on line 1543, ExecuteTeardown running on another thread can Dispose() the semaphore. WaitAsync() then throws ObjectDisposedException. Because the method is async void, the exception unwinds onto the ambient SynchronizationContext (none, so the default TaskScheduler) and surfaces as an unhandled TaskScheduler.UnobservedTaskException / crash, rather than being logged by the method's own catch block further down.
It is a narrow race — you need a child-exit event to fire during teardown — but in a Windows Service the shutdown path is exactly when child-exit events are most likely (SCM stopping → child tree collapses).
Suggested fix:
Move the await inside the outer try so the existing catch handler covers it:
private async void OnProcessExited(object sender, EventArgs e)
{
try
{
if (_isTearingDown || _disposed) return;
_logger?.Warn("Child process exit detected via event.");
ClearProcessState();
bool needsRecovery = false;
bool shouldStop = false;
int exitCode = -1;
await _healthCheckSemaphore.WaitAsync();
try
{
...
}
finally
{
_healthCheckSemaphore.Release();
}
}
catch (ObjectDisposedException)
{
// Teardown raced us; nothing to do.
}
catch (Exception ex)
{
_logger?.Error($"OnProcessExited failed: {ex.Message}", ex);
}
}
Severity: Warning
File:
src/Servy.Service/Service.cslines 1531-1546Description:
OnProcessExitedis the Exited-event handler and is declaredasync void(required for event-handler signatures). The current body awaits_healthCheckSemaphore.WaitAsync()before the try block starts:#473(closed) correctly moved this from blockingWait()toWaitAsync(), but left the await outside the exception-handling boundary.Failure mode: the
(_isTearingDown || _disposed)check on line 1533 is not synchronized with teardown — between that check and theawaiton line 1543,ExecuteTeardownrunning on another thread canDispose()the semaphore.WaitAsync()then throwsObjectDisposedException. Because the method isasync void, the exception unwinds onto the ambientSynchronizationContext(none, so the default TaskScheduler) and surfaces as an unhandledTaskScheduler.UnobservedTaskException/ crash, rather than being logged by the method's own catch block further down.It is a narrow race — you need a child-exit event to fire during teardown — but in a Windows Service the shutdown path is exactly when child-exit events are most likely (SCM stopping → child tree collapses).
Suggested fix:
Move the
awaitinside the outer try so the existing catch handler covers it: