Skip to content

[Robustness] MonitoringViewModelBase.OnTick — async void handler does not wrap OnTickAsync in try/catch; an unhandled exception will crash the WPF dispatcher #982

@Christophe-Rogiers

Description

@Christophe-Rogiers

Severity: Warning

File: src/Servy.Manager/ViewModels/MonitoringViewModelBase.cs

Lines: 80-106

Code:

private async void OnTick(object sender, EventArgs e)
{
    if (Interlocked.CompareExchange(ref _isMonitoringFlag, 1, 1) == 0 ||
        Interlocked.CompareExchange(ref _isTickRunningFlag, 1, 0) == 1)
    {
        return;
    }

    _timer?.Stop();

    try
    {
        await OnTickAsync();
    }
    finally
    {
        Interlocked.Exchange(ref _isTickRunningFlag, 0);

        if (Interlocked.CompareExchange(ref _isMonitoringFlag, 1, 1) == 1)
        {
            _timer?.Start();
        }
    }
}

Explanation:

OnTick is the standard async-void event handler for DispatcherTimer.Tick. It uses try / finally purely for cleanup, but not to catch exceptions thrown by OnTickAsync(). Subclasses include PerformanceViewModel, ConsoleViewModel, LogsViewModel, etc., and any of their OnTickAsync overrides can throw — processHelper.GetProcessTreeMetrics can throw Win32Exception, Repository.SearchAsync can throw SQLiteException, the Event Log query in EventLogService.SearchAsync can throw InvalidOperationException / SecurityException (per its own catch-and-rethrow), etc.

Async-void handlers do not propagate exceptions back to the dispatcher synchronously — they're posted to the synchronization context as unobserved exceptions and bring the WPF Manager UI down with TaskScheduler.UnobservedTaskException → process termination. From an operator's perspective, the Manager simply disappears the next time a polling tick fails.

The companion code in ServiceSearchViewModelBase.SearchServicesAsync (lines 124-156) already does the right thing: a catch (Exception ex) { Logger.Error(...); } block around the await. The monitoring base needs the same.

Suggested fix:

Wrap the await in a try/catch so the ticker becomes self-healing:

try
{
    await OnTickAsync();
}
catch (OperationCanceledException) { /* expected during shutdown */ }
catch (Exception ex)
{
    Logger.Error($"Monitoring tick failed in {GetType().Name}.", ex);
    // Do NOT rethrow — async void would terminate the dispatcher.
}
finally { ... }

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions