diff --git a/src/PlanViewer.App/MainWindow.FileOps.cs b/src/PlanViewer.App/MainWindow.FileOps.cs index a206a1f..933b98f 100644 --- a/src/PlanViewer.App/MainWindow.FileOps.cs +++ b/src/PlanViewer.App/MainWindow.FileOps.cs @@ -583,9 +583,18 @@ internal List CollectOpenTabEntries() /// /// Saves the restore entries of all currently open tabs — file paths, and since #496 /// scratch buffer entries — docked and detached alike (#490). + /// + /// Not in a secondary instance (): + /// the list on disk is the owner's, and this is the one writer that would replace it + /// with the secondary's own tabs. Every write of the list comes through here — the + /// debounce, the final write in , and the update restart — so the + /// one guard covers all three. /// private void SaveOpenPlans() { + if (SingleInstance.IsSecondaryInstance) + return; + _appSettings.OpenTabs.Clear(); _appSettings.OpenTabs.AddRange(CollectOpenTabEntries()); @@ -627,6 +636,12 @@ timer against a window being torn down. */ if (IsShuttingDown) return; + /* A secondary instance writes no session state (see SaveOpenPlans), so there is + nothing to schedule: no pending flag, and no timer that would tick just to find + the write refused. */ + if (SingleInstance.IsSecondaryInstance) + return; + _sessionPersistPending = true; /* No real timer under the test host, in #451's pattern: the suite shares one @@ -741,6 +756,11 @@ internal void PersistSessionForRestart() /// the poison defense wholesale — the entry is already off the cleared list before its /// buffer is read, and a buffer that fails to load is skipped, never re-added, and /// deleted (). + /// + /// Never in a secondary instance. Everything above is written for the one + /// instance that owns the saved session: the clear-and-save empties the owner's list on + /// disk, and the sweep deletes buffers the owner still needs. + /// skips this method when is set. /// /// /// Whether an empty restore opens a fresh query tab. False when the caller is about to diff --git a/src/PlanViewer.App/MainWindow.ScratchPersist.cs b/src/PlanViewer.App/MainWindow.ScratchPersist.cs index 3433e82..10519a8 100644 --- a/src/PlanViewer.App/MainWindow.ScratchPersist.cs +++ b/src/PlanViewer.App/MainWindow.ScratchPersist.cs @@ -99,6 +99,16 @@ different questions (staleness against on-disk changes, most of all). That is #4 /// private void HookScratchPersistence(QuerySessionControl session) { + /* A secondary instance never persists scratch content. The buffer folder and the ids + in it belong to the instance that owns the slot: writing there would put this + window's text under names the owner also writes, drops and sweeps. With no + subscription nothing is ever queued, no buffer id is minted, and every place that + drops a buffer finds no id and does nothing — the drop sites need no guard of + their own. The cost is crash recovery for this window's scratch tabs; the close + prompts do not depend on it. */ + if (SingleInstance.IsSecondaryInstance) + return; + /* Only sessions that are scratch NOW. SourceFilePath moves null→path exactly once (the save) and never back, so a session arriving here file-backed can never need this hook later — the scope fence again, applied at subscription time. It also diff --git a/src/PlanViewer.App/MainWindow.axaml.cs b/src/PlanViewer.App/MainWindow.axaml.cs index 24b3d23..ab7494d 100644 --- a/src/PlanViewer.App/MainWindow.axaml.cs +++ b/src/PlanViewer.App/MainWindow.axaml.cs @@ -70,7 +70,12 @@ public MainWindow() // Not in the test host (#451): every test window would grab the machine's single // SQLPerformanceStudio_OpenFile pipe slot and never release it — OnClosed never // runs there — racing any real Studio instance on the same box. - if (!AppRuntimeMode.IsTestHost) + // Not in a secondary instance either: the pipe belongs to the instance that owns the + // slot. The listener retries until it gets the pipe, so a secondary would take it once + // the owner exits, and later launches would hand their files to a window whose tabs are + // never saved. Without it, such a launch finds no pipe, claims the slot, and runs as the + // new owner. + if (!AppRuntimeMode.IsTestHost && !SingleInstance.IsSecondaryInstance) StartPipeServer(); InitializeComponent(); @@ -224,7 +229,23 @@ this only changes the cold start. The restore's new-tab fallback stays out of th way when a file is about to open — a stray empty scratch tab beside the file the user asked for is nobody's intent. */ var hasFileArg = args.Length > 1 && File.Exists(args[1]); - RestoreOpenPlans(createFallbackTab: !hasFileArg); + + if (SingleInstance.IsSecondaryInstance) + { + /* A secondary instance (--new-instance beside a running one) does not restore. + The saved list belongs to the instance that owns the slot, which rewrites it on + every tab change: restoring it here would open a copy of every one of its tabs + and share its scratch buffer ids. RestoreOpenPlans is skipped whole, not called + with a flag, because it also clears and saves the list and sweeps the buffer + folder — both of which would land on the owner's files. What is left is what a + launch with nothing to restore does: the file it was given, else a new tab. */ + if (!hasFileArg) + NewQuery_Click(this, new RoutedEventArgs()); + } + else + { + RestoreOpenPlans(createFallbackTab: !hasFileArg); + } if (hasFileArg) OpenFileByExtension(args[1]); diff --git a/src/PlanViewer.App/Program.cs b/src/PlanViewer.App/Program.cs index 6e9f916..f932e35 100644 --- a/src/PlanViewer.App/Program.cs +++ b/src/PlanViewer.App/Program.cs @@ -50,7 +50,10 @@ other setting (AtomicFile prevents torn writes, not lost updates). File-argument launches already forwarded to the running instance over the pipe; a BARE second launch ran a full instance and was exactly the clobber case. So unless the user explicitly asks for a second instance, a launch that finds one running hands it - its work — a file path, or a bare "surface yourself" — and exits. */ + its work — a file path, or a bare "surface yourself" — and exits. A launch that + does ask (--new-instance) still gets its own window, but it claims the slot first + and, if another instance already holds it, runs as a secondary that leaves the + saved session alone — see ClaimSlotForNewInstance. */ var newInstanceRequested = SingleInstance.NewInstanceRequested(args); // The flag is a launcher directive, not a file: strip it so nothing downstream can @@ -101,6 +104,11 @@ when the loser's retries run out before the winner's pipe exists. The pre-#489 last-write-wins behavior, which is the accepted floor. */ } } + else + { + // --new-instance: never handed to a running instance, but it still claims the slot. + ClaimSlotForNewInstance(); + } BuildAvaloniaApp() .StartWithClassicDesktopLifetime(effectiveArgs); @@ -118,16 +126,40 @@ public static AppBuilder BuildAvaloniaApp() .WithInterFont() .LogToTrace(); + /// + /// What --new-instance does about the slot. It never hands its launch to the + /// running instance — the user asked for a window of their own — but it no longer skips + /// the slot either. It claims it the same way an ordinary launch does: + /// + /// Slot free: no other instance is running, so this launch is an ordinary one. It + /// restores, persists and owns the slot, and later launches hand their work to it. + /// Slot held: another instance owns the saved session, so this process runs as a + /// secondary () and leaves that session + /// alone. Before this, it restored the owner's list into a second window, and the two + /// windows then wrote, dropped and swept the same scratch buffers. + /// + /// Split from so the decision can be tested with a mutex name of the + /// test's own; the harness cannot start a second app process to hold the real one. + /// + /// Where named mutexes do not work at all, + /// answers true so that the launch still runs, and this launch then acts as an owner: it + /// restores the saved session even if another instance is running. That is how every + /// --new-instance launch behaved before, and it only happens where the mutex + /// machinery itself fails. + /// + internal static void ClaimSlotForNewInstance(string mutexName = SingleInstance.MutexName) => + SingleInstance.IsSecondaryInstance = !TryBecomeSingleInstanceOwner(mutexName); + /// /// Tries to claim the single-instance slot (#489). True means this process is the /// owner and should run; false means another instance holds the slot and this launch - /// should hand its work over instead. + /// should hand its work over instead (or, for --new-instance, run as a secondary). /// - private static bool TryBecomeSingleInstanceOwner() + private static bool TryBecomeSingleInstanceOwner(string mutexName = SingleInstance.MutexName) { try { - var mutex = new Mutex(initiallyOwned: true, SingleInstance.MutexName, out var createdNew); + var mutex = new Mutex(initiallyOwned: true, mutexName, out var createdNew); if (createdNew) { _singleInstanceMutex = mutex; diff --git a/src/PlanViewer.App/Services/AppSettingsService.cs b/src/PlanViewer.App/Services/AppSettingsService.cs index f1b4d8b..d8f8b40 100644 --- a/src/PlanViewer.App/Services/AppSettingsService.cs +++ b/src/PlanViewer.App/Services/AppSettingsService.cs @@ -193,6 +193,18 @@ public static AppSettings Load() /// while found the file unreadable, which would otherwise overwrite /// content this process has never actually seen. The Settings window reports that case /// through . + /// + /// In a secondary instance () the + /// saved open-tab list is kept as the disk has it. The whole file is written, so the + /// settings object's list goes with it — and a secondary's copy is the one it read at its + /// own startup, long out of date next to the list the owner rewrites on every tab change. + /// Recent plans, the Settings dialog and the server-filter toggle all save through here. + /// The list is read off the disk just before the write, so the write hands back what the + /// owner last saved. An owner save that lands between that read and the write is still + /// lost; nothing locks the file across processes, and the gap is milliseconds. Every other + /// setting stays last-write-wins, which is what two instances on purpose has always meant + /// (see Program.Main). If the file cannot be read there is no list to keep, so the save is + /// skipped rather than written with a guess. /// public static void Save(AppSettings settings) { @@ -201,6 +213,9 @@ public static void Save(AppSettings settings) try { + if (SingleInstance.IsSecondaryInstance && !TryAdoptSavedOpenTabs(settings)) + return; + Directory.CreateDirectory(SettingsDir); var json = JsonSerializer.Serialize(settings, JsonOptions); AtomicFile.WriteAllText(SettingsPath, json); @@ -212,6 +227,32 @@ public static void Save(AppSettings settings) } } + /// + /// Puts the open-tab list that is on disk right now into , for a + /// secondary instance about to write the whole file (see ). Read the way + /// reads, so a file that will not parse is moved aside rather than + /// overwritten. False when the file exists but cannot be read — the caller must not write. + /// + private static bool TryAdoptSavedOpenTabs(AppSettings settings) + { + var outcome = SettingsFileStore.Read( + SettingsPath, + nameof(AppSettingsService), + json => JsonSerializer.Deserialize(json, JsonOptions), + out var onDisk); + + if (outcome == SettingsFileStore.ReadOutcome.Unreadable) + return false; + + if (onDisk != null) + MigrateOpenTabs(onDisk); + + // Missing, or unparseable and just moved aside: there is no list on disk to keep, and + // an empty one is a truer thing to write than a stale one. + settings.OpenTabs = onDisk?.OpenTabs ?? new List(); + return true; + } + /// /// Moves an "open_plans" list written by an older version onto , /// so upgrading does not cost the user the tabs they had open. Only fills an empty OpenTabs: diff --git a/src/PlanViewer.App/SingleInstance.cs b/src/PlanViewer.App/SingleInstance.cs index 2bd53ad..2eb1ea0 100644 --- a/src/PlanViewer.App/SingleInstance.cs +++ b/src/PlanViewer.App/SingleInstance.cs @@ -47,12 +47,42 @@ internal static class SingleInstance internal const string MutexName = "SQLPerformanceStudio_SingleInstance"; /// - /// Escape hatch (#489): skip the single-instance check and run a full second instance. - /// A user who runs two on purpose accepts settings last-write-wins as their informed - /// choice. Stripped from argv before any file-open logic sees it. + /// Escape hatch (#489): open a window of its own instead of handing the launch to the + /// running instance. A user who runs two on purpose accepts settings last-write-wins as + /// their informed choice — but not a duplicated session: the launch claims the slot + /// first, and when another instance already holds it this process runs as a secondary + /// (see ). Stripped from argv before any file-open + /// logic sees it. /// internal const string NewInstanceFlag = "--new-instance"; + /// + /// True in a process started with while another instance + /// already owned the single-instance slot. Set once by before any + /// window exists; nothing else in the product sets it. Tests that set it belong in the + /// serial collection, because it changes what every settings save writes. + /// + /// Why a secondary must not own the session. The saved open-tab list and the + /// scratch buffer folder have one writer by design: the owner rewrites the list on every + /// tab change, names buffers by ids only it knows, and sweeps any buffer its own list + /// does not name. A second process that restored that list would open a copy of every + /// tab, share the owner's buffer ids so that both windows write, drop and sweep the same + /// files, and whichever window closed last would overwrite the other's list. + /// + /// What a secondary does instead. It restores nothing (a file argument still + /// opens; otherwise the usual new tab), never writes the list or a scratch buffer, never + /// sweeps the buffer folder, and keeps the list already on disk when it saves settings + /// (see AppSettingsService.Save). What it loses is session restore for its own + /// tabs: they are not reopened at the next start, whether it closed cleanly or crashed, + /// and its scratch tabs have no crash recovery. The unsaved-changes prompts do not depend + /// on persistence and work as they always have. + /// + /// Not set on the launch path where a non-owner runs fully after the pipe hand-off + /// failed (see Program.Main): that launch never asked for a second window, so it + /// keeps the pre-#489 behavior. + /// + internal static bool IsSecondaryInstance { get; set; } + /// /// The line a bare second launch sends to mean "surface your main window". The /// double-colons make it unrepresentable as a Windows file name on purpose — see the diff --git a/tests/PlanViewer.Core.Tests/SecondaryInstanceTests.cs b/tests/PlanViewer.Core.Tests/SecondaryInstanceTests.cs new file mode 100644 index 0000000..bfb89fe --- /dev/null +++ b/tests/PlanViewer.Core.Tests/SecondaryInstanceTests.cs @@ -0,0 +1,550 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text.Json; +using System.Threading; +using Avalonia.Controls; +using Avalonia.Interactivity; +using Avalonia.Threading; +using PlanViewer.App; +using PlanViewer.App.Controls; +using PlanViewer.App.Services; + +namespace PlanViewer.Core.Tests; + +/// +/// #489 review: a window started with --new-instance beside a running Studio used to +/// restore the running one's saved open-tab list, so it opened a copy of every tab, shared the +/// running window's scratch buffer ids (both then wrote, dropped and swept the same files), and +/// whichever window closed last overwrote the other's list. --new-instance now claims the +/// single-instance slot first; when another instance holds it, this process is a secondary. A +/// secondary restores nothing, never writes the list or a scratch buffer, never sweeps the +/// buffer folder, and keeps the list already on disk when it saves settings. It loses crash +/// recovery for its own tabs and nothing else. +/// +/// What "leaves it alone" is checked against. Every secondary test stages what a +/// running owner leaves on disk — a list with a file and a scratch entry, that scratch's buffer, +/// and an old orphaned buffer that an ordinary start would sweep — and compares the settings file +/// and the buffer folder before and after, content AND last-write time, so a write of identical +/// bytes still fails. The test host arms no real timers, so each test drives the flushes itself +/// (, +/// ) and closes the window through +/// its real close path: a test that only waited would pass for the wrong reason. +/// +/// Why this joins the serial collection. +/// is process-wide and changes what writes, the same kind of +/// hazard as the save block the collection was made for. A test in another class that saved +/// settings while it was on would have its open-tab list replaced with the one on disk. +/// +[Collection("SettingsFileStore serial")] +public class SecondaryInstanceTests +{ + private const string OwnerScratchText = "SELECT 2 AS owner_scratch;"; + + // ── the secondary ───────────────────────────────────────────────────── + + [Fact] + public void ASecondaryWindowRestoresNoneOfTheSavedListAndLeavesItAlone() + { + AsSecondaryOverOwnerSession((window, owner) => + { + /* Starting up alone is enough to have touched them: an ordinary start clears the list + and saves it empty before it opens anything, then sweeps the buffer folder. */ + AssertUnchanged(owner); + + var sessions = Sessions(window).ToList(); + Assert.DoesNotContain(sessions, s => s.SourceFilePath == owner.SqlPath); + Assert.DoesNotContain(sessions, s => s.ScratchBufferId == owner.ScratchId); + Assert.DoesNotContain(sessions, s => s.QueryEditor.Text == OwnerScratchText); + + /* What a launch with nothing to restore does: the usual new tab, empty and clean. */ + var only = Assert.Single(window.MainTabControl.Items.OfType()); + var session = Assert.IsType(only.Content); + Assert.Null(session.SourceFilePath); + Assert.False(session.IsDirty); + + /* Nothing pending anywhere, and a forced flush of both writers stays inert. */ + window.FlushPendingSessionPersistForTests(); + window.FlushPendingScratchPersistForTests(); + }); + } + + [Fact] + public void ASecondaryStillOpensItsFileArgumentAndNothingElse() + { + AsSecondaryOverOwnerSession((window, owner) => + { + var fileArg = TempSql("SELECT 1 AS launched_with_this;"); + try + { + var tabsBefore = window.MainTabControl.Items.Count; // the constructor's usual new tab + + window.OpenFromStartupArgs(new[] { "PerformanceStudio.exe", "--new-instance", fileArg }); + + var sessions = Sessions(window).ToList(); + Assert.Single(sessions, s => s.SourceFilePath == fileArg); + Assert.Equal(tabsBefore + 1, window.MainTabControl.Items.Count); // the file, no fallback tab + Assert.DoesNotContain(sessions, s => s.SourceFilePath == owner.SqlPath); + Assert.DoesNotContain(sessions, s => s.ScratchBufferId == owner.ScratchId); + } + finally + { + File.Delete(fileArg); + } + }); + } + + [Fact] + public void ScratchTabsInASecondaryNeverTouchTheSavedListOrTheBufferFolder() + { + AsSecondaryOverOwnerSession((window, _) => + { + var first = NewScratchTab(window, "SELECT 1 AS typed_in_secondary;"); + window.FlushPendingScratchPersistForTests(); + + first.QueryEditor.Text = "SELECT 1 AS edited_in_secondary;"; + window.FlushPendingScratchPersistForTests(); + + NewScratchTab(window, "SELECT 2 AS second_scratch;"); + window.FlushPendingSessionPersistForTests(); + window.PersistSessionForRestart(); // the update-restart write + + /* Close the first tab through the real button and answer Don't Save: the moment an + ordinary window deletes the buffer it wrote for that tab. */ + CloseButton(TabOf(window, first)).RaiseEvent(new RoutedEventArgs(Button.ClickEvent)); + Dispatcher.UIThread.RunJobs(); + AnswerDontSave(window); + Assert.DoesNotContain(first, Sessions(window)); + window.FlushPendingScratchPersistForTests(); + + /* Then close the window with the second tab still dirty: the walk asks, Don't Save + lets it through, and OnClosed runs its final drain and list write. */ + window.Close(); + Dispatcher.UIThread.RunJobs(); + AnswerDontSave(window); + Assert.False(window.IsVisible); + }); + } + + [Fact] + public void ASecondaryStillAsksAboutADirtyScratchTabWhenItCloses() + { + AsSecondaryOverOwnerSession((window, _) => + { + var session = NewScratchTab(window, "SELECT 1 AS unsaved_in_secondary;"); + window.FlushPendingScratchPersistForTests(); // persistence is off, so no buffer holds this text + + Assert.True(window.CloseNeedsConfirmation()); + + window.Close(); + Dispatcher.UIThread.RunJobs(); + + Assert.True(window.IsVisible, "the close is held while the question is up"); + var prompt = Assert.Single(window.OwnedWindows); + + /* Dismissing the prompt is Cancel: the window stays and the text is still there. */ + prompt.Close(); + Dispatcher.UIThread.RunJobs(); + Assert.True(window.IsVisible); + Assert.Equal("SELECT 1 AS unsaved_in_secondary;", session.QueryEditor.Text); + + window.Close(); + Dispatcher.UIThread.RunJobs(); + AnswerDontSave(window); + Assert.False(window.IsVisible); + }); + } + + [Fact] + public void ASecondarysSettingsSaveKeepsTheListThatIsOnDisk() + { + AsSecondaryOverOwnerSession((window, owner) => + { + /* The owner keeps working after this window started: it opens another tab and + rewrites the list. Written straight to the file, as another process would, so this + process's cached settings still hold the list they read at startup. */ + var opened = Path.Combine(Path.GetTempPath(), "opened_by_the_owner_later.sql"); + var later = new List + { + owner.SqlPath, + ScratchBufferStore.EntryFor(owner.ScratchId), + opened, + }; + var ownerCopy = AppSettingsService.Load().Clone(); + ownerCopy.OpenTabs = later; + File.WriteAllText(AppSettingsService.SettingsFilePath, JsonSerializer.Serialize(ownerCopy)); + + /* A save the secondary makes for its own reasons: opening a plan puts it on the + Recent Plans list, which writes the whole settings file. */ + var plan = Path.Combine(AppContext.BaseDirectory, "Plans", "row_goal_plan.sqlplan"); + window.LoadPlanFile(plan); + + var saved = ReadSettingsFile(); + Assert.Equal(later, saved.OpenTabs); + Assert.Contains(Path.GetFullPath(plan), saved.RecentPlans); // the save itself did happen + }, + filesMustStay: false); + } + + [Fact] + public void ASecondaryThatCannotReadTheSavedListDoesNotSave() + { + Assert.SkipUnless(OperatingSystem.IsWindows(), + "Unix permissions don't reliably block a same-user read the way FileShare.None does on Windows."); + + var owner = StageOwnerSession(); + var path = AppSettingsService.SettingsFilePath; + var settings = AppSettingsService.Load(); + var originalDays = settings.QueryStoreSlicerDays; + try + { + var before = File.ReadAllBytes(path); + settings.QueryStoreSlicerDays = 91; + + SingleInstance.IsSecondaryInstance = true; + using (new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.None)) + { + AppSettingsService.Save(settings); // must not throw + } + SingleInstance.IsSecondaryInstance = false; + + /* Not written, and not even attempted: an attempt would have staged a .tmp + sibling and then failed on the rename. With no way to see the list there is no + list to keep, and a guess is not written. */ + Assert.True(before.AsSpan().SequenceEqual(File.ReadAllBytes(path))); + Assert.False(File.Exists(path + ".tmp")); + } + finally + { + SingleInstance.IsSecondaryInstance = false; + settings.QueryStoreSlicerDays = originalDays; + File.Delete(path + ".tmp"); + ResetState(owner); + } + } + + // ── the slot claim ──────────────────────────────────────────────────── + + [Fact] + public void ANewInstanceLaunchBesideAnOwnerRunsAsASecondary() + { + var name = UniqueMutexName(); + using var runningOwner = new Mutex(initiallyOwned: true, name, out var held); + Assert.True(held); + + try + { + PlanViewer.App.Program.ClaimSlotForNewInstance(name); + + Assert.True(SingleInstance.IsSecondaryInstance); + } + finally + { + SingleInstance.IsSecondaryInstance = false; + } + } + + [Fact] + public void ANewInstanceLaunchWithNoOtherOwnerClaimsTheSlotAndRestoresNormally() + { + var name = UniqueMutexName(); + + HeadlessUi.Run(() => + { + var owner = StageOwnerSession(); + MainWindow? window = null; + try + { + PlanViewer.App.Program.ClaimSlotForNewInstance(name); + + Assert.False(SingleInstance.IsSecondaryInstance, "the slot was free, so this launch owns it"); + + /* And it holds the slot now: the next launch finds the name taken and hands its + work over, which is the whole point of claiming it. */ + using (var probe = new Mutex(initiallyOwned: false, name, out var created)) + Assert.False(created); + + window = new MainWindow(); + window.Show(); + AssertOwnsTheSavedSession(window, owner); + } + finally + { + SingleInstance.IsSecondaryInstance = false; + PutAway(window); + ResetState(owner); + } + }); + } + + [Fact] + public void ANormalLaunchIsUnchanged() + { + HeadlessUi.Run(() => + { + var owner = StageOwnerSession(); + MainWindow? window = null; + try + { + Assert.False(SingleInstance.IsSecondaryInstance, "nothing sets the flag on an ordinary launch"); + + window = new MainWindow(); + window.Show(); + AssertOwnsTheSavedSession(window, owner); + } + finally + { + PutAway(window); + ResetState(owner); + } + }); + } + + // ── plumbing ────────────────────────────────────────────────────────── + + /// + /// What a running owner leaves on disk, as far as these tests care, and the settings file and + /// buffer folder exactly as it left them (see ). + /// + private sealed record OwnerSession( + string SqlPath, + Guid ScratchId, + Guid OrphanId, + SortedDictionary OnDisk); + + /// + /// The whole secondary scenario, through the same door production uses: a slot already held + /// by "the running instance" (a mutex of this test's own name), a --new-instance + /// launch claiming it and finding it taken, and then a window built under the flag that + /// leaves behind. The window is closed while still a secondary, and unless a test says + /// otherwise the owner's files must come out exactly as they went in. + /// + private static void AsSecondaryOverOwnerSession( + Action body, bool filesMustStay = true) + { + HeadlessUi.Run(() => + { + var owner = StageOwnerSession(); + MainWindow? window = null; + try + { + var name = UniqueMutexName(); + using var runningOwner = new Mutex(initiallyOwned: true, name, out var held); + Assert.True(held); + PlanViewer.App.Program.ClaimSlotForNewInstance(name); + Assert.True(SingleInstance.IsSecondaryInstance, "the slot is held, so this launch is a secondary"); + + try + { + window = new MainWindow(); + window.Show(); + body(window, owner); + } + finally + { + PutAway(window); + SingleInstance.IsSecondaryInstance = false; + } + + if (filesMustStay) + AssertUnchanged(owner); + } + finally + { + ResetState(owner); + } + }); + } + + /// + /// What ordinary startup did before and still does: restore the list in place, sweep debris, + /// keep the restored scratch's buffer, and keep persisting new scratch content and the list. + /// + private static void AssertOwnsTheSavedSession(MainWindow window, OwnerSession owner) + { + var sessions = Sessions(window).ToList(); + Assert.Equal(2, window.MainTabControl.Items.Count); // the file and the scratch, no extra new tab + Assert.Single(sessions, s => s.SourceFilePath == owner.SqlPath); + + var scratch = Assert.Single(sessions, s => s.ScratchBufferId == owner.ScratchId); + Assert.Equal(OwnerScratchText, scratch.QueryEditor.Text); + Assert.True(scratch.IsDirty); + + Assert.True(File.Exists(ScratchBufferStore.BufferPathFor(owner.ScratchId))); + Assert.False(File.Exists(ScratchBufferStore.BufferPathFor(owner.OrphanId)), + "an ordinary start sweeps a buffer nothing lists"); + Assert.Equal( + new[] { owner.SqlPath, ScratchBufferStore.EntryFor(owner.ScratchId) }, + ReadSettingsFile().OpenTabs); + + var typed = NewScratchTab(window, "SELECT 9 AS typed_by_the_owner;"); + window.FlushPendingScratchPersistForTests(); + + var id = typed.ScratchBufferId; + Assert.NotNull(id); + Assert.Equal( + "SELECT 9 AS typed_by_the_owner;", + File.ReadAllText(ScratchBufferStore.BufferPathFor(id!.Value))); + Assert.Contains(ScratchBufferStore.EntryFor(id.Value), ReadSettingsFile().OpenTabs); + } + + /// + /// Writes what a running owner leaves behind: the list (a file tab, then a scratch tab), the + /// scratch tab's buffer, and one buffer nothing lists that is old enough for a start-up sweep + /// to delete. Done in ordinary mode, before any secondary exists, and left in the shared + /// cache as well as on disk, which is where the next window reads its list from. + /// + private static OwnerSession StageOwnerSession() + { + var sqlPath = TempSql("SELECT 1 AS owner_file;"); + var scratchId = Guid.NewGuid(); + var orphanId = Guid.NewGuid(); + + ScratchBufferStore.Write(scratchId, OwnerScratchText); + ScratchBufferStore.Write(orphanId, "SELECT 3 AS orphan;"); + File.SetLastWriteTimeUtc( + ScratchBufferStore.BufferPathFor(orphanId), + DateTime.UtcNow - ScratchBufferStore.OrphanGracePeriod - TimeSpan.FromDays(1)); + + var settings = AppSettingsService.Load(); + settings.OpenTabs.Clear(); + settings.OpenTabs.Add(sqlPath); + settings.OpenTabs.Add(ScratchBufferStore.EntryFor(scratchId)); + AppSettingsService.Save(settings); + + return new OwnerSession(sqlPath, scratchId, orphanId, Snapshot()); + } + + /// Leaves nothing for the next test's window to restore or sweep. + private static void ResetState(OwnerSession? owner) + { + SingleInstance.IsSecondaryInstance = false; // the save below has to be an ordinary one + + var settings = AppSettingsService.Load(); + settings.OpenTabs.Clear(); + AppSettingsService.Save(settings); + + foreach (var file in ScratchFiles()) + { + try + { + File.Delete(file); + } + catch + { + // Best-effort — a stuck file is swept by a later window anyway. + } + } + + if (owner != null) + File.Delete(owner.SqlPath); + } + + /// + /// The settings file and every file in the scratch folder, with the time each was last + /// written. Content alone would let a rewrite of identical bytes through. + /// + private static SortedDictionary Snapshot() + { + var snapshot = new SortedDictionary(StringComparer.Ordinal); + + void Add(string key, string path) => + snapshot[key] = (File.ReadAllBytes(path), File.GetLastWriteTimeUtc(path).Ticks); + + Add("appsettings.json", AppSettingsService.SettingsFilePath); + foreach (var file in ScratchFiles()) + Add("scratch/" + Path.GetFileName(file), file); + + return snapshot; + } + + /// The settings file and buffer folder are exactly as the owner left them. + private static void AssertUnchanged(OwnerSession owner) + { + var after = Snapshot(); + + Assert.Equal(owner.OnDisk.Keys.ToArray(), after.Keys.ToArray()); + foreach (var (name, was) in owner.OnDisk) + { + Assert.True(was.Bytes.AsSpan().SequenceEqual(after[name].Bytes), $"{name} changed"); + Assert.True(was.Written == after[name].Written, $"{name} was written to"); + } + } + + private static AppSettings ReadSettingsFile() => + JsonSerializer.Deserialize(File.ReadAllText(AppSettingsService.SettingsFilePath))!; + + private static string[] ScratchFiles() => + Directory.Exists(AppSettingsService.ScratchDirectory) + ? Directory.GetFiles(AppSettingsService.ScratchDirectory) + : Array.Empty(); + + /// + /// A name of this test's own, so nothing here can meet the real slot on a developer's + /// machine, or another run's. Same reasoning as SingleInstanceTests' mutex self-test. + /// + private static string UniqueMutexName() => + $"{SingleInstance.MutexName}_secondary_{Guid.NewGuid():N}"; + + private static QuerySessionControl NewScratchTab(MainWindow window, string text) + { + window.NewQuery_Click(window, new RoutedEventArgs()); + var session = Sessions(window).Last(); + session.QueryEditor.Text = text; + return session; + } + + /// Clicks Don't Save on the one prompt the window is asking. + private static void AnswerDontSave(MainWindow window) + { + PromptButton(Assert.Single(window.OwnedWindows), "Don't Save") + .RaiseEvent(new RoutedEventArgs(Button.ClickEvent)); + Dispatcher.UIThread.RunJobs(); + } + + /// + /// Prompts closed, every session settled, window shut — from a finally, because the run + /// where it matters is the run where an assertion above it failed (#474). + /// + private static void PutAway(MainWindow? window) + { + if (window == null || !window.IsVisible) + return; + + foreach (var prompt in window.OwnedWindows.ToList()) + prompt.Close(); + + foreach (var session in Sessions(window).ToList()) + session.MarkClean(); + + Dispatcher.UIThread.RunJobs(); + window.Close(); + Dispatcher.UIThread.RunJobs(); + } + + private static TabItem TabOf(MainWindow window, QuerySessionControl session) => + window.MainTabControl.Items.OfType().Single(t => t.Content == session); + + private static Button CloseButton(TabItem tab) => + ((StackPanel)tab.Header!).Children.OfType [CollectionDefinition("SettingsFileStore serial", DisableParallelization = true)] public class SettingsFileStoreSerialCollection