From 29de816df10a97fc16de6a55d5208184ca010d9b Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 3 Sep 2026 09:08:24 -0400 Subject: [PATCH] Pin the detached window's close-reentry latch too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #488 gave DetachedWindowHelper the same walk-in-progress latch MainWindow.OnClosing got, for the same gap, but only the MainWindow half was pinned. Deleting the detached closeGuardPending check outright left the suite green — which is how a latch gets tidied away by someone who reasonably believes the tests are watching it. Same shape as the existing test, against the detached window: close once and the question comes up, close again and no second prompt stacks, dismiss it and the window stays with the latch cleared, close once more and a fresh question starts. Proved red against the latch's absence before it went green. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016a1AnKAHwcALrwdYVVrpgR --- .../WindowCloseReentryTests.cs | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/tests/PlanViewer.Core.Tests/WindowCloseReentryTests.cs b/tests/PlanViewer.Core.Tests/WindowCloseReentryTests.cs index 4c374d9..a0952e2 100644 --- a/tests/PlanViewer.Core.Tests/WindowCloseReentryTests.cs +++ b/tests/PlanViewer.Core.Tests/WindowCloseReentryTests.cs @@ -14,6 +14,12 @@ namespace PlanViewer.Core.Tests; /// used to start a second concurrent walk: duplicate prompts about the same tab, and a Cancel /// answered to one walk that the other never heard. Same reentrancy class as the About /// window's update link (#485 review), and the same walk-in-progress latch closes it. +/// +/// Both guards, not one. DetachedWindowHelper carries its own copy of the +/// latch for its own copy of the gap, and the two are separate code with separate state. Only +/// the MainWindow half was pinned when the latch landed: deleting the detached +/// closeGuardPending check outright left this class green, which is how a latch gets +/// tidied away by someone who reasonably believes the tests are watching it. /// public class WindowCloseReentryTests { @@ -80,6 +86,80 @@ public void ASecondCloseDuringTheWalkDoesNotStartASecondWalk() }); } + [Fact] + public void ASecondCloseOfADetachedWindowDoesNotStackASecondPrompt() + { + HeadlessUi.Run(() => + { + var path = TempSql("SELECT 1;"); + MainWindow? window = null; + Window? detached = null; + QuerySessionControl? session = null; + try + { + window = new MainWindow(); + window.Show(); + window.LoadSqlFile(path); + + var tab = window.MainTabControl.Items.OfType() + .Last(t => t.Content is QuerySessionControl); + session = (QuerySessionControl)tab.Content!; + session.QueryEditor.Text = "SELECT 2; -- unsaved work"; + + detached = window.DetachTabToWindow(tab)!; + Dispatcher.UIThread.RunJobs(); + + detached.Close(); + Dispatcher.UIThread.RunJobs(); + + Assert.True(detached.IsVisible, "the close is held while the question is up"); + Assert.Single(detached.OwnedWindows); + + /* The second X. The guard's prompt is modal to the detached window, but a Save As + raised from that prompt is not, so this is reachable in the real app. */ + detached.Close(); + Dispatcher.UIThread.RunJobs(); + + Assert.Single(detached.OwnedWindows); + Assert.True(detached.IsVisible); + + /* Dismissing is Cancel: the window stays, and the latch has to clear with it or + the X is dead for the rest of that window's life. */ + detached.OwnedWindows.Single().Close(); + Dispatcher.UIThread.RunJobs(); + + Assert.True(detached.IsVisible); + Assert.Empty(detached.OwnedWindows); + + detached.Close(); + Dispatcher.UIThread.RunJobs(); + + Assert.Single(detached.OwnedWindows); // a fresh close starts a fresh question + } + finally + { + if (detached != null) + { + foreach (var prompt in detached.OwnedWindows.ToList()) + prompt.Close(); + session?.MarkClean(); + Dispatcher.UIThread.RunJobs(); + detached.Close(); + Dispatcher.UIThread.RunJobs(); + } + + if (window != null) + { + session?.MarkClean(); + window.Close(); + Dispatcher.UIThread.RunJobs(); + } + + File.Delete(path); + } + }); + } + private static string TempSql(string text) { var path = Path.Combine(Path.GetTempPath(), $"{Path.GetRandomFileName()}.sql");