From cd798745c2c141724f9c027d757f0d6104fb0984 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Fri, 21 Aug 2026 20:56:48 +1000 Subject: [PATCH] Let the viewer close when Windows is shutting down OnFormClosing cancelled every close, whatever the CloseReason. For a close the user asked for that is right: whether closing means hide or exit is ViewerProgram's rule and it needs a tray check to decide, so the form defers and CloseForReal brings the answer back. For the close Windows sends when the session is ending it is not. WinForms answers WM_QUERYENDSESSION with !e.Cancel, so a viewer that was open at shutdown told Windows it was preventing one, and the user got the "DiffEngineViewer is preventing shutdown" screen. With a tray running it was worse than a prompt: the loop's answer to a close request is to hide the window, so the process stayed up and went on blocking until the user chose "Shut down anyway". Do not cancel when the reason is WindowsShutDown or TaskManagerClosing. Both mean the process is going away regardless, so refusing buys nothing. Letting the close through is safe: the loop already watches for a disposed form and returns, running the same shutdown it would have run anyway. --- .../GlobalUsings.cs | 2 +- .../ViewerFormClosingTests.cs | 53 +++++++++++++++++++ src/DiffEngineViewer.Windows/ViewerForm.cs | 20 +++++-- 3 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 src/DiffEngineViewer.Windows.Tests/ViewerFormClosingTests.cs diff --git a/src/DiffEngineViewer.Windows.Tests/GlobalUsings.cs b/src/DiffEngineViewer.Windows.Tests/GlobalUsings.cs index 0b4fd261..bc3662c4 100644 --- a/src/DiffEngineViewer.Windows.Tests/GlobalUsings.cs +++ b/src/DiffEngineViewer.Windows.Tests/GlobalUsings.cs @@ -1,2 +1,2 @@ global using System.Buffers.Binary; -global using System.IO.Compression; \ No newline at end of file +global using System.IO.Compression;global using System.Reflection; diff --git a/src/DiffEngineViewer.Windows.Tests/ViewerFormClosingTests.cs b/src/DiffEngineViewer.Windows.Tests/ViewerFormClosingTests.cs new file mode 100644 index 00000000..df610c12 --- /dev/null +++ b/src/DiffEngineViewer.Windows.Tests/ViewerFormClosingTests.cs @@ -0,0 +1,53 @@ +/// +/// Who gets to refuse a close. +/// +/// The form cancels a user close because whether closing means hide or exit is ViewerProgram's +/// rule, not the form's. It used to cancel every close, including the one Windows sends when the +/// session is ending — and WinForms answers WM_QUERYENDSESSION with !e.Cancel, so the viewer +/// reported itself as preventing shutdown. +/// +/// +/// Driven through OnFormClosing by reflection, because CloseReason is set by the message that +/// started the close and there is no way to ask a form to close as though Windows had. +/// +/// +[NotInParallel] +[TUnit.Core.Executors.STAThreadExecutor] +public class ViewerFormClosingTests +{ + [Test] + [Arguments(CloseReason.UserClosing, true)] + [Arguments(CloseReason.None, true)] + [Arguments(CloseReason.WindowsShutDown, false)] + [Arguments(CloseReason.TaskManagerClosing, false)] + public async Task Cancels(CloseReason reason, bool expected) + { + using var form = new ViewerForm("title", 800, 600); + var args = new FormClosingEventArgs(reason, false); + + Raise(form, args); + + await Assert.That(args.Cancel).IsEqualTo(expected); + } + + /// + /// And CloseForReal still wins, whatever the reason, since that is the loop answering its own + /// question. + /// + [Test] + public async Task CloseForRealIsNeverCancelled() + { + using var form = new ViewerForm("title", 800, 600); + form.CloseForReal(); + var args = new FormClosingEventArgs(CloseReason.UserClosing, false); + + Raise(form, args); + + await Assert.That(args.Cancel).IsFalse(); + } + + static void Raise(ViewerForm form, FormClosingEventArgs args) => + typeof(ViewerForm) + .GetMethod("OnFormClosing", BindingFlags.Instance | BindingFlags.NonPublic)! + .Invoke(form, [args]); +} diff --git a/src/DiffEngineViewer.Windows/ViewerForm.cs b/src/DiffEngineViewer.Windows/ViewerForm.cs index b497800e..fe3a3456 100644 --- a/src/DiffEngineViewer.Windows/ViewerForm.cs +++ b/src/DiffEngineViewer.Windows/ViewerForm.cs @@ -312,9 +312,16 @@ public void CloseForReal() protected override void OnFormClosing(FormClosingEventArgs e) { - // Always cancelled, because whether closing means hide or exit is ViewerProgram's rule and - // it needs a tray check to decide. CloseForReal is how the answer comes back. - if (!closingForReal) + // Cancelled for a close the user asked for, because whether that means hide or exit is + // ViewerProgram's rule and it needs a tray check to decide. CloseForReal is how the answer + // comes back. + // + // Never for a close the session is ending: WinForms answers WM_QUERYENDSESSION with + // !e.Cancel, so refusing made Windows report the viewer as preventing shutdown, and with a + // tray running the loop only hid the window - leaving the process blocking until the user + // chose "Shut down anyway". Letting it through is safe because the loop watches for a + // disposed form and returns, which runs the same shutdown it would have run anyway. + if (!closingForReal && !EndsTheSession(e.CloseReason)) { closeRequested = true; e.Cancel = true; @@ -323,6 +330,13 @@ protected override void OnFormClosing(FormClosingEventArgs e) base.OnFormClosing(e); } + /// + /// The process is going away whatever this form says. Task Manager's End Task is here with + /// shutdown because refusing it buys the same nothing: the user has already decided. + /// + internal static bool EndsTheSession(CloseReason reason) => + reason is CloseReason.WindowsShutDown or CloseReason.TaskManagerClosing; + /// /// ProcessCmdKey rather than OnKeyDown, because Tab and Escape are consumed by focus /// navigation and the default button before a key handler would ever see them.