WPF test - #30
WPF test#30
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new WPF desktop sample to demonstrate MinimalWorker background workers with a simple live “dashboard” UI.
Changes:
- Introduces a new
MinimalWorker.Desktop.Wpf.Sampleproject (WPF UI + view models). - Adds worker state tracking (
WorkerStateService/WorkerInfo) and binds it to the UI. - Updates
MinimalWorker.slnto include the new WPF sample project.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/MinimalWorker.Desktop.Wpf.Sample/WorkerStateService.cs | Tracks per-worker state and raises update notifications. |
| samples/MinimalWorker.Desktop.Wpf.Sample/WorkerInfo.cs | Stores mutable worker metrics used by the dashboard. |
| samples/MinimalWorker.Desktop.Wpf.Sample/ViewModels/WorkerCardViewModel.cs | View model for a single worker “card” in the UI. |
| samples/MinimalWorker.Desktop.Wpf.Sample/ViewModels/MainWindowViewModel.cs | Main dashboard VM (uptime + per-worker cards). |
| samples/MinimalWorker.Desktop.Wpf.Sample/MinimalWorker.Desktop.Wpf.Sample.csproj | New WPF sample project configuration and references. |
| samples/MinimalWorker.Desktop.Wpf.Sample/MainWindow.xaml.cs | WPF window code-behind. |
| samples/MinimalWorker.Desktop.Wpf.Sample/MainWindow.xaml | Dashboard UI layout and bindings. |
| samples/MinimalWorker.Desktop.Wpf.Sample/App.xaml.cs | WPF app bootstrap: host creation, worker registration, lifecycle shutdown. |
| samples/MinimalWorker.Desktop.Wpf.Sample/App.xaml | Application resources (theme brushes). |
| MinimalWorker.sln | Adds the new WPF sample project to the solution. |
| private int _executionCount; | ||
|
|
||
| public string Name { get; } = name; | ||
| public int ExecutionCount => _executionCount; | ||
| public DateTime? LastRunTime { get; private set; } | ||
| public string StatusMessage { get; private set; } = "Waiting..."; | ||
|
|
||
| internal void Record(string message) | ||
| { | ||
| Interlocked.Increment(ref _executionCount); | ||
| LastRunTime = DateTime.Now; | ||
| StatusMessage = message; |
There was a problem hiding this comment.
Record updates LastRunTime/StatusMessage and _executionCount from background worker threads, while the UI reads these values. Because the fields are updated without synchronization, cross-thread readers can observe stale/inconsistent state. Consider making the update/read thread-safe (e.g., swap an immutable snapshot atomically, or use Volatile.Read/Write / a lock for the mutable fields).
| private int _executionCount; | |
| public string Name { get; } = name; | |
| public int ExecutionCount => _executionCount; | |
| public DateTime? LastRunTime { get; private set; } | |
| public string StatusMessage { get; private set; } = "Waiting..."; | |
| internal void Record(string message) | |
| { | |
| Interlocked.Increment(ref _executionCount); | |
| LastRunTime = DateTime.Now; | |
| StatusMessage = message; | |
| private readonly object _syncRoot = new(); | |
| private int _executionCount; | |
| private DateTime? _lastRunTime; | |
| private string _statusMessage = "Waiting..."; | |
| public string Name { get; } = name; | |
| public int ExecutionCount | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _executionCount; | |
| } | |
| } | |
| } | |
| public DateTime? LastRunTime | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _lastRunTime; | |
| } | |
| } | |
| private set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _lastRunTime = value; | |
| } | |
| } | |
| } | |
| public string StatusMessage | |
| { | |
| get | |
| { | |
| lock (_syncRoot) | |
| { | |
| return _statusMessage; | |
| } | |
| } | |
| private set | |
| { | |
| lock (_syncRoot) | |
| { | |
| _statusMessage = value; | |
| } | |
| } | |
| } | |
| internal void Record(string message) | |
| { | |
| lock (_syncRoot) | |
| { | |
| _executionCount++; | |
| _lastRunTime = DateTime.Now; | |
| _statusMessage = message; | |
| } |
| public void RecordExecution(string workerName, string message) | ||
| { | ||
| var info = GetOrAdd(workerName); | ||
| info.Record(message); | ||
| StateChanged?.Invoke(workerName); |
There was a problem hiding this comment.
The StateChanged event only passes workerName, which causes subscribers to do another GetOrAdd lookup to retrieve the updated info. Consider raising an event that includes the updated WorkerInfo (or an immutable snapshot) so subscribers can avoid the extra lookup and reduce races between update and read.
| protected override async void OnStartup(StartupEventArgs e) | ||
| { | ||
| base.OnStartup(e); | ||
|
|
||
| var builder = Host.CreateApplicationBuilder(e.Args); | ||
| builder.Services.AddSingleton<WorkerStateService>(); | ||
|
|
||
| _host = builder.Build(); | ||
|
|
There was a problem hiding this comment.
OnStartup is async void (WPF requirement), so any exception thrown after an await will crash the process and is difficult to observe. Wrap the async body in try/catch (log + Shutdown(-1)), so startup failures (e.g., host build/start) fail predictably.
| protected override async void OnExit(ExitEventArgs e) | ||
| { | ||
| if (_host is not null) | ||
| { | ||
| await _host.StopAsync(); | ||
| _host.Dispose(); | ||
| } |
There was a problem hiding this comment.
In OnExit, consider awaiting StopAsync with ConfigureAwait(false) (and optionally a timeout) so shutdown doesn’t depend on resuming on the Dispatcher synchronization context while the app is already exiting.
No description provided.