From eed96e1a1c5f3c57411293fdd3ee56316c385ef0 Mon Sep 17 00:00:00 2001 From: BlackSpirits Date: Tue, 15 Sep 2026 22:31:52 +0200 Subject: [PATCH] Cancel FFmpeg library work when dialog closes --- .../Shared/DownloadFfmpegLibsViewModel.cs | 74 ++++++++++++++--- .../DownloadFfmpegLibsViewModelTests.cs | 79 +++++++++++++++++++ 2 files changed, 142 insertions(+), 11 deletions(-) create mode 100644 tests/UI/Features/Shared/DownloadFfmpegLibsViewModelTests.cs diff --git a/src/ui/Features/Shared/DownloadFfmpegLibsViewModel.cs b/src/ui/Features/Shared/DownloadFfmpegLibsViewModel.cs index dc50c534ccb..1644a98e16a 100644 --- a/src/ui/Features/Shared/DownloadFfmpegLibsViewModel.cs +++ b/src/ui/Features/Shared/DownloadFfmpegLibsViewModel.cs @@ -36,7 +36,8 @@ public partial class DownloadFfmpegLibsViewModel : ObservableObject, IClosingCle private readonly IFfmpegLibsDownloadService _downloadService; private Task? _downloadTask; private readonly Timer _timer; - private bool _done; + private volatile bool _done; + private int _cleanupStarted; private readonly CancellationTokenSource _cancellationTokenSource; private IndeterminateProgressHelper? _indeterminateProgressHelper; private readonly Lock _lockObj = new(); @@ -93,14 +94,7 @@ private void OnTimerOnElapsed(object? sender, ElapsedEventArgs args) } finally { - try - { - File.Delete(_tempFileName); - } - catch - { - // temp file, best effort - } + TryDeleteTempFile(_tempFileName); } StopIndeterminateProgress(); @@ -138,6 +132,11 @@ internal static void ExtractLibraries(string zipFileName, string targetFolder, C private void StartIndeterminateProgress() { + if (_cancellationTokenSource.IsCancellationRequested) + { + return; + } + _indeterminateProgressHelper?.Dispose(); _indeterminateProgressHelper = new IndeterminateProgressHelper( value => ProgressValue = value, @@ -159,14 +158,67 @@ private void Close() [RelayCommand] private void CommandCancel() { - _cancellationTokenSource.Cancel(); - _done = true; + CancelPendingWork(); Close(); } + private void CancelPendingWork() + { + _done = true; + _cancellationTokenSource.Cancel(); + StopIndeterminateProgress(); + } + public void OnClosingCleanup() { + if (Interlocked.Exchange(ref _cleanupStarted, 1) != 0) + { + return; + } + + // Closed can come from the Cancel button, Escape, the title-bar X, or normal success. + // Make the first three equivalent: stop all work before detaching the polling timer. + CancelPendingWork(); _timer.StopAndDispose(OnTimerOnElapsed); + _ = DeleteTempFileWhenTaskCompletesAsync(_downloadTask, _tempFileName); + } + + internal static async Task DeleteTempFileWhenTaskCompletesAsync(Task? downloadTask, string tempFileName) + { + if (downloadTask != null) + { + try + { + await downloadTask.ConfigureAwait(false); + } + catch + { + // Cancellation/download failure is already the reason cleanup is running. + } + } + + TryDeleteTempFile(tempFileName); + } + + private static void TryDeleteTempFile(string tempFileName) + { + if (string.IsNullOrWhiteSpace(tempFileName)) + { + return; + } + + try + { + if (File.Exists(tempFileName)) + { + File.Delete(tempFileName); + } + } + catch + { + // Best effort. During unpacking the timer callback may still own the ZIP and its + // finally block will retry deletion once that operation observes cancellation. + } } public void StartDownload() diff --git a/tests/UI/Features/Shared/DownloadFfmpegLibsViewModelTests.cs b/tests/UI/Features/Shared/DownloadFfmpegLibsViewModelTests.cs new file mode 100644 index 00000000000..80c6fdcca4c --- /dev/null +++ b/tests/UI/Features/Shared/DownloadFfmpegLibsViewModelTests.cs @@ -0,0 +1,79 @@ +using Nikse.SubtitleEdit.Features.Shared; +using Nikse.SubtitleEdit.Logic.Download; + +namespace UITests.Features.Shared; + +public class DownloadFfmpegLibsViewModelTests +{ + [Fact] + public void OnClosingCleanup_CancelsPendingDownload() + { + var service = new BlockingFfmpegLibsDownloadService(); + var vm = new DownloadFfmpegLibsViewModel(service); + + vm.StartDownload(); + Assert.True(service.Started.Wait(2000, TestContext.Current.CancellationToken)); + Assert.False(service.CapturedToken.IsCancellationRequested); + + vm.OnClosingCleanup(); + + Assert.True(service.CapturedToken.IsCancellationRequested); + } + + [Fact] + public async Task DeleteTempFileWhenTaskCompletesAsync_WaitsThenDeletes() + { + var fileName = Path.Combine(Path.GetTempPath(), "se-ffmpeg-close-" + Guid.NewGuid().ToString("N") + ".zip"); + File.WriteAllText(fileName, "partial"); + var completion = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + try + { + var cleanup = DownloadFfmpegLibsViewModel.DeleteTempFileWhenTaskCompletesAsync(completion.Task, fileName); + Assert.False(cleanup.IsCompleted); + Assert.True(File.Exists(fileName)); + + completion.SetResult(); + await cleanup; + + Assert.False(File.Exists(fileName)); + } + finally + { + File.Delete(fileName); + } + } + + [Fact] + public async Task DeleteTempFileWhenTaskCompletesAsync_FaultStillDeletes() + { + var fileName = Path.Combine(Path.GetTempPath(), "se-ffmpeg-close-" + Guid.NewGuid().ToString("N") + ".zip"); + File.WriteAllText(fileName, "partial"); + + try + { + await DownloadFfmpegLibsViewModel.DeleteTempFileWhenTaskCompletesAsync( + Task.FromException(new IOException("download failed")), + fileName); + + Assert.False(File.Exists(fileName)); + } + finally + { + File.Delete(fileName); + } + } + + private sealed class BlockingFfmpegLibsDownloadService : IFfmpegLibsDownloadService + { + internal ManualResetEventSlim Started { get; } = new(false); + internal CancellationToken CapturedToken { get; private set; } + + public async Task DownloadFfmpegLibs(string destinationFileName, IProgress? progress, CancellationToken cancellationToken) + { + CapturedToken = cancellationToken; + Started.Set(); + await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken); + } + } +}