Skip to content

Make long running steps cancellable and keep the queue from stalling the UI - #3

Merged
Kentarohakase merged 3 commits into
mainfrom
hardening/cancellation-and-ui-responsiveness
Aug 12, 2026
Merged

Make long running steps cancellable and keep the queue from stalling the UI#3
Kentarohakase merged 3 commits into
mainfrom
hardening/cancellation-and-ui-responsiveness

Conversation

@Kentarohakase

Copy link
Copy Markdown
Owner

Stacked on #2. Phase 2 of the improvement plan.

Fixed

  • Adding files left the window half usable. LoadInputFilesAsync guarded on IsBusy but never raised it. While a dropped folder was analyzed, Cancel was disabled, Start was enabled and further drops were accepted. It now runs as a busy phase with its own cancellation source, so the analysis can actually be stopped. PrepareRetryItemsAsync follows the same shape.
  • The processed preview render could not be stopped. It is a full FFmpeg pass over the loudest section and ran with CancellationToken.None. It now runs as a busy phase too — which is what makes Cancel reach it at all — and reports a cancellation as a normal outcome.
  • A cancelled analysis would have crashed the app. MainWindow_OnDrop is async void with no guard, so an escaping OperationCanceledException would have reached the dispatcher handler. Both entry points handle it themselves.
  • The startup tasks were fire and forget. PrepareYtDlpAsync and CheckForAppUpdateAsync were started with _ = …, never tracked and never cancelled. They are held in fields now, cancelled on shutdown, and check the token before writing view model state again. Their catch-all no longer hides real defects: a cancellation stays silent, anything else is logged.

Changed

  • The queue rebuilt everything on every progress tick. OnBatchItemPropertyChanged called UpdateBatchSummary() and RaiseCommandStates() for every property of every item, including Progress, which is written on each FFmpeg tick. RaiseCommandStates makes WPF re-evaluate 24 predicates synchronously on the UI thread — several do file system calls, three allocate an array over the whole queue, and two build a complete filter plan. The refresh is now limited to the properties that actually feed the summary counts and the command predicates (Status, ComparisonReport, HasComparisonWarnings, OutputPath).
  • The tool source was a German literal. Benutzer-Tools, App-Ordner and Tools-Ordner were built in ToolDiscoveryService and handed straight to the app bar, so an English user read German labels in an otherwise translated interface. ToolLocation now carries a resource key that ToolStatus.DisplayText resolves.
  • A broken tool was probed again on every call. Only successful statuses were cached. With the bounded probe from Keep failing commands, slow tools and untrusted update links from breaking the session #2 that costs the full 20 seconds each time. Failures are cached too now, but only for 30 seconds, so a tool installed while the app runs is still picked up.

Two deviations from the plan

  • The semaphore dispose race does not exist. The plan listed a fix for AnalyzeBatchItemsAsync disposing its SemaphoreSlim while releasers were still running. Task.WhenAll waits for all tasks to complete, not just until the first one fails, and every task releases in its own finally before completing — so the using never disposes early. No fix was needed and none was made.
  • Dispose cancels the startup tasks but does not await them. They resume on the dispatcher, so blocking that thread while it has to run their continuations would deadlock. Cancelling plus a token check before each state write achieves the same end without the deadlock.

Verification

dotnet format .\AudioQualityEnhancer.slnx --verify-no-changes   -> clean
dotnet build  .\AudioQualityEnhancer.slnx -c Release            -> 0 warnings, 0 errors
dotnet test   .\AudioQualityEnhancer.slnx -c Release            -> 204 passed, 0 failed

The UI behaviour is not covered by tests and needs a manual pass: drop a folder with many files and check Cancel works; start a batch export and check the window stays responsive; switch the language to English and check the tool source in the app bar.

…stalling the UI

Adding files never raised IsBusy, so while a dropped folder was analyzed Cancel
stayed disabled, Start stayed enabled and further drops were accepted. The
analysis now runs as a busy phase with its own cancellation source, and so does
the processed preview render, which is a full FFmpeg pass and was equally
unstoppable. Both report a cancellation as a normal outcome instead of letting
it escape into the drop handler, which has no guard of its own.

The two startup tasks were fire and forget. They are held now and cancelled on
shutdown, and they check the token before writing view model state again, so a
late answer can no longer raise change notifications into a torn down view.
Their catch-all no longer hides a genuine defect: a cancellation stays silent,
anything else is logged. Waiting for them in Dispose would deadlock the thread
that has to run their continuations, so only the cancellation is issued.

Finally, every property change of every queue item rebuilt the summary and
re-raised all 24 command states. Progress is written on every FFmpeg tick and
feeds neither, so the refresh is now limited to the properties that do.
The place a tool was found in was built as a German literal and handed straight
to the app bar, so an English user read Benutzer-Tools next to an otherwise
translated interface. The location now carries a resource key that is resolved
on display.

A failed probe was never cached. Since the probe is bounded by a timeout, a
missing or unresponsive binary cost the full twenty seconds on every call. The
failure is cached as well now, but only for half a minute, so a tool that is
installed while the app runs is still picked up.
@Kentarohakase
Kentarohakase force-pushed the hardening/cancellation-and-ui-responsiveness branch from 56e3df3 to b7f755d Compare August 12, 2026 01:01
@Kentarohakase
Kentarohakase changed the base branch from claude/code-review-improvements-gk09et to main August 12, 2026 01:01
@Kentarohakase Kentarohakase reopened this Aug 12, 2026
@Kentarohakase
Kentarohakase merged commit 2076f2a into main Aug 12, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant