From 0b6059d4a8948ba096bb1d73a2ad651addfbb964 Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Tue, 4 Aug 2026 17:56:39 +0200 Subject: [PATCH] Stop a late progress report from putting the download bar back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Progress` does not invoke its callback inline — it posts to the captured synchronisation context — so a report can be delivered after the download it belongs to has already returned. The unguarded handler then wrote a stale smaller number over the 100 set on completion, and nothing reports again, so it stayed there: a finished download showing 50% next to a banner offering the restart. Caught by running the suite a second time on another checkout, which is the only reason it was caught at all. The assertion that failed is the one that says a found update is fetched without being asked about, and it had passed on every previous run — the ordering it depends on is real and simply usually goes the other way. Guarded rather than made synchronous. The posting is wanted: in the application these callbacks arrive on whichever thread Velopack downloads on, and setting an observable property off the UI thread is a binding exception rather than a stale number. Monotonic is what the bar should have been anyway, since progress that goes backwards is not progress. Ran five times over to confirm it is settled rather than merely rearranged. --- .../ViewModels/UpdateViewModel.cs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs index 520ad33..413cc5c 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs @@ -346,7 +346,22 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab State = UpdateState.Downloading; DownloadPercent = 0; - var progress = new Progress(percent => DownloadPercent = percent); + // Monotonic, and it has to be. Progress delivers its callbacks by posting them to the captured + // synchronisation context rather than invoking them inline, so a report can arrive after the + // download has already returned — and an unguarded assignment then puts a stale smaller number + // back on the bar and leaves it there, because nothing reports again. Seen for real: a run of this + // finished at 50 with the state already Ready. + // + // Guarding here rather than reaching for an inline IProgress, because the posting is wanted: in the + // application these callbacks come off whichever thread the updater downloads on, and an observable + // property changed off the UI thread is a binding exception rather than a stale number. + var progress = new Progress(percent => + { + if (percent > DownloadPercent) + { + DownloadPercent = percent; + } + }); await updates.DownloadAsync(found, progress, cancellationToken).ConfigureAwait(true);