From 7c9069d0d7d2f3b435a4ec0f6d3571f39883e095 Mon Sep 17 00:00:00 2001 From: jschick04 Date: Fri, 24 Jul 2026 18:55:43 +0000 Subject: [PATCH] Commit a completed events export even if Cancel is clicked late MauiMenuActionService.ExportEventsAsync passed the export's cancellation token to SaveStreamingAsync, so AtomicFileWriter's post-write cancellation gate could throw between the streaming write finishing and the atomic commit. A Cancel click landing in that window discarded a fully written export and reported it canceled. A `finished` flag tried to disarm the banner Cancel after the write, but it was set a few instructions too late to close the race. Scope cancellation to the write phase instead: pass the CTS token directly to EventTableExporter.ExportAsync (which throws per row on Cancel, so the temp is discarded mid-write) and pass CancellationToken.None to SaveStreamingAsync so the atomic commit is unconditional once the write completes. A late Cancel is now a no-op and the completed export is saved, matching Windows Event Viewer and the two other SaveStreamingAsync callers that already pass None. The banner Cancel callback swallows ObjectDisposedException to stay safe against the CTS being disposed during teardown, replacing the racy `finished` flag. Lock the two guarantees at the Runtime layer (the MAUI action has no test harness): EventTableExporter aborts and writes nothing on a canceled token, and AtomicFileWriter commits a completed write under CancellationToken.None even when a separate source was canceled. --- .../Adapters/Menu/MauiMenuActionService.cs | 25 ++++++++----------- .../Common/Files/AtomicFileWriterTests.cs | 20 +++++++++++++++ .../Export/EventTableExporterTests.cs | 15 +++++++++++ 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/EventLogExpert/Adapters/Menu/MauiMenuActionService.cs b/src/EventLogExpert/Adapters/Menu/MauiMenuActionService.cs index 42410ae9..416ee476 100644 --- a/src/EventLogExpert/Adapters/Menu/MauiMenuActionService.cs +++ b/src/EventLogExpert/Adapters/Menu/MauiMenuActionService.cs @@ -182,7 +182,6 @@ await _dialogService.ShowAlert( } CancellationTokenSource cancellation = new(); - bool finished = false; string? savedPath = null; Exception? failure = null; bool canceled = false; @@ -192,22 +191,22 @@ await _dialogService.ShowAlert( savedPath = await _fileSaveService.SaveStreamingAsync( suggestedFileName, fileTypes, - async (stream, token) => + async (stream, _) => { - // Begin only once the picker has returned a destination and the streaming write is - // starting; dismissing the picker never reaches here, so it stays silent. The finished - // flag turns a late Cancel click into a clean no-op. + // Begin only once the picker has returned a destination and the streaming write is starting; + // dismissing the picker never reaches here, so it stays silent. A Cancel click can race export + // teardown (the CTS is disposed in the finally), so guard the narrow disposed-CTS window. _exportProgress.Begin( - "Exporting events...", () => { if (!Volatile.Read(ref finished)) { cancellation.Cancel(); } }); + "Exporting events...", + () => { try { cancellation.Cancel(); } catch (ObjectDisposedException) { /* Teardown disposed the CTS; a late Cancel is a no-op. */ } }); + // Cancellation gates the WRITE only: ExportAsync throws per-row on Cancel, so AtomicFileWriter + // discards the temp. SaveStreamingAsync gets CancellationToken.None, so once the write completes + // the atomic commit is unconditional - a late Cancel can no longer discard a fully-written export. await _eventTableExporter.ExportAsync( - stream, format, events, columns, timeZone, includeDescription: true, token); - - // The streaming write is done; stop honoring Cancel before SaveStreamingAsync runs its - // post-write cancel gates, so a late Cancel can't abort an already-written export's commit. - Volatile.Write(ref finished, true); + stream, format, events, columns, timeZone, includeDescription: true, cancellation.Token); }, - cancellation.Token); + CancellationToken.None); } catch (OperationCanceledException) when (cancellation.IsCancellationRequested) { @@ -219,8 +218,6 @@ await _eventTableExporter.ExportAsync( } finally { - Volatile.Write(ref finished, true); - // End() raises StateChanged; isolate it so a throwing subscriber can never skip the CTS // disposal or the in-flight reset, which would otherwise wedge every later export. try diff --git a/tests/Unit/EventLogExpert.Runtime.Tests/Common/Files/AtomicFileWriterTests.cs b/tests/Unit/EventLogExpert.Runtime.Tests/Common/Files/AtomicFileWriterTests.cs index c1726d7e..a59d06e8 100644 --- a/tests/Unit/EventLogExpert.Runtime.Tests/Common/Files/AtomicFileWriterTests.cs +++ b/tests/Unit/EventLogExpert.Runtime.Tests/Common/Files/AtomicFileWriterTests.cs @@ -107,6 +107,26 @@ public async Task WriteAsync_NoDirectoryComponent_ThrowsArgumentException() => await Assert.ThrowsAnyAsync( () => AtomicFileWriter.WriteAsync("barefilename.log", WriteText("x"), TestContext.Current.CancellationToken)); + [Fact] + public async Task WriteAsync_NoneToken_CommitsCompletedWriteEvenWhenASeparateSourceIsCancelled() + { + // Mirrors ExportEventsAsync: the write is scoped to a caller CTS, but AtomicFileWriter receives None, so a + // Cancel that lands after the content is written cannot abort the atomic commit - the completed file is saved. + var destination = Destination(); + using var cancellation = new CancellationTokenSource(); + + Func writeThenCancelSeparate = async (stream, _) => + { + await stream.WriteAsync(Encoding.UTF8.GetBytes("exported"), TestContext.Current.CancellationToken); + await cancellation.CancelAsync(); + }; + + await AtomicFileWriter.WriteAsync(destination, writeThenCancelSeparate, CancellationToken.None); + + Assert.Equal("exported", await File.ReadAllTextAsync(destination, TestContext.Current.CancellationToken)); + Assert.Empty(LeakedTempFiles()); + } + [Fact] public async Task WriteAsync_NullWriteContent_ThrowsArgumentNullException() => await Assert.ThrowsAsync( diff --git a/tests/Unit/EventLogExpert.Runtime.Tests/Export/EventTableExporterTests.cs b/tests/Unit/EventLogExpert.Runtime.Tests/Export/EventTableExporterTests.cs index 8413f1da..20fc3fca 100644 --- a/tests/Unit/EventLogExpert.Runtime.Tests/Export/EventTableExporterTests.cs +++ b/tests/Unit/EventLogExpert.Runtime.Tests/Export/EventTableExporterTests.cs @@ -21,6 +21,21 @@ public sealed class EventTableExporterTests ]; private static readonly TimeZoneInfo s_utc = TimeZoneInfo.Utc; + [Fact] + public async Task ExportAsync_CancelledToken_ThrowsAndWritesNothing() + { + using var cancellation = new CancellationTokenSource(); + await cancellation.CancelAsync(); + EventTableExporter exporter = new(new TabularExportWriter()); + using MemoryStream stream = new(); + + // A Cancel during the export must throw so AtomicFileWriter discards the temp; nothing is written. + await Assert.ThrowsAnyAsync(() => exporter.ExportAsync( + stream, ExportFormat.Csv, BuildView(s_events), s_columns, s_utc, includeDescription: false, cancellation.Token)); + + Assert.Equal(0, stream.Length); + } + [Fact] public async Task ExportAsync_Csv_ExcludeDescription_OmitsColumn() {