From 39cd4ffc9c02a6dbae0d2b0ea2f2a2c03671eb70 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 22 Aug 2026 14:54:30 +1000 Subject: [PATCH] Give a remote discard the time an accept gets Discard and DiscardAll used ViewerClient.ShortTimeout, which is 500ms and exists for calls on a clock - the tray's scan timer, where waiting the full timeout would let callbacks outlast their own period. Discarding is not one of those: it comes from the menu or a hot key, and it no longer runs on the UI thread. Meanwhile the owner answering it may be inside InlineApplier, which waits up to ten seconds on its cross process mutex. So half a second turned a busy owner into "The snapshot viewer is not running." for a single discard. DiscardAll was worse, because its result was dropped. Tracker.Clear emptied its own snapshot list regardless, so "Discard (n)" reported success while the owner had never received the message - and everything reappeared on the next scan two seconds later. It now returns the outcome, and Clear only forgets what it holds when the owner confirms. Both wait on acceptWait, the same fifteen seconds an accept gets, for the same reason. --- .../OwnedInlineHostTest.cs | 2 +- src/DiffEngineTray.Tests/StubInlineHost.cs | 4 +++- src/DiffEngineTray/IInlineHost.cs | 6 +++++- src/DiffEngineTray/OwnedInlineHost.cs | 8 ++++++-- src/DiffEngineTray/RemoteInlineHost.cs | 18 +++++++++++++++--- src/DiffEngineTray/Tracker.cs | 13 +++++++++++-- 6 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs b/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs index 61529c66..b6a31f7c 100644 --- a/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs +++ b/src/DiffEngineTray.Tests/OwnedInlineHostTest.cs @@ -315,7 +315,7 @@ public async Task DiscardAllEmptiesTheQueue() owner.Queue(); owner.Queue(@"c:\repo\OtherTests.cs", 7); - owner.Host.DiscardAll(); + owner.Host.DiscardAll(out _); await Assert.That(owner.Host.List()).IsEmpty(); } diff --git a/src/DiffEngineTray.Tests/StubInlineHost.cs b/src/DiffEngineTray.Tests/StubInlineHost.cs index 3f6c0f89..852a1cd1 100644 --- a/src/DiffEngineTray.Tests/StubInlineHost.cs +++ b/src/DiffEngineTray.Tests/StubInlineHost.cs @@ -59,8 +59,10 @@ public bool AcceptAll(out string? message) return AcceptAllSucceeds; } - public void DiscardAll() + public bool DiscardAll(out string? message) { + message = null; + return true; } public void Focus(PendingSnapshot snapshot) diff --git a/src/DiffEngineTray/IInlineHost.cs b/src/DiffEngineTray/IInlineHost.cs index 8ceec101..ca9c5156 100644 --- a/src/DiffEngineTray/IInlineHost.cs +++ b/src/DiffEngineTray/IInlineHost.cs @@ -26,7 +26,11 @@ interface IInlineHost bool Discard(PendingSnapshot snapshot, out string? message); bool AcceptAll(out string? message); - void DiscardAll(); + /// + /// False when the queue owner could not be asked, so a caller clearing its own state knows not + /// to. + /// + bool DiscardAll(out string? message); /// /// Bring the window forward on this item, launching one if there is none. diff --git a/src/DiffEngineTray/OwnedInlineHost.cs b/src/DiffEngineTray/OwnedInlineHost.cs index 4b394629..997b6c53 100644 --- a/src/DiffEngineTray/OwnedInlineHost.cs +++ b/src/DiffEngineTray/OwnedInlineHost.cs @@ -143,8 +143,12 @@ public bool AcceptAll(out string? message) } } - public void DiscardAll() => - ((IQueueOwner) this).DiscardAll(); + public bool DiscardAll(out string? message) + { + message = ((IQueueOwner) this).DiscardAll(); + // Owned in this process, so there is nobody to fail to reach + return true; + } public void Focus(PendingSnapshot snapshot) => Show(WindowCommand.Focus, snapshot.Key); diff --git a/src/DiffEngineTray/RemoteInlineHost.cs b/src/DiffEngineTray/RemoteInlineHost.cs index 26397e5b..f89c8152 100644 --- a/src/DiffEngineTray/RemoteInlineHost.cs +++ b/src/DiffEngineTray/RemoteInlineHost.cs @@ -96,8 +96,14 @@ public AcceptOutcome Accept(PendingSnapshot snapshot, out string? message) : AcceptOutcome.Applied; } + /// + /// On , not the short timeout. Discarding is not a clock driven call + /// - it comes from the menu or a hot key - and the owner answering it may be busy inside + /// InlineApplier, which waits up to ten seconds on its cross process mutex. Half a second + /// turned a busy owner into "The snapshot viewer is not running." + /// public bool Discard(PendingSnapshot snapshot, out string? message) => - Send(ViewerVerb.Discard, snapshot.Key, ViewerClient.ShortTimeout, out message); + Send(ViewerVerb.Discard, snapshot.Key, acceptWait, out message); /// /// True only when the queue is empty afterwards, for the reason gives — @@ -110,8 +116,14 @@ public bool AcceptAll(out string? message) => Send(ViewerVerb.AcceptAll, null, acceptWait, out message) && List().Count == 0; - public void DiscardAll() => - Send(ViewerVerb.DiscardAll, null, ViewerClient.ShortTimeout, out _); + /// + /// As , and the outcome is returned rather than dropped. Discarded on a + /// busy owner used to do nothing at all while Tracker.Clear went ahead and emptied its own + /// snapshot list, so "Discard (n)" reported success and everything reappeared on the next + /// scan two seconds later. + /// + public bool DiscardAll(out string? message) => + Send(ViewerVerb.DiscardAll, null, acceptWait, out message); public void Focus(PendingSnapshot snapshot) => Send(ViewerVerb.Focus, snapshot.Key, ViewerClient.ShortTimeout, out _); diff --git a/src/DiffEngineTray/Tracker.cs b/src/DiffEngineTray/Tracker.cs index 5ab19434..fc9aba8f 100644 --- a/src/DiffEngineTray/Tracker.cs +++ b/src/DiffEngineTray/Tracker.cs @@ -671,8 +671,17 @@ public void Clear() { ((ITrackedFiles) this).DiscardAll(); - inline.DiscardAll(); - snapshots = []; + // Only forget the cached snapshots when the owner actually discarded them. It used to be + // cleared regardless, so a discard the owner never received still emptied the menu - and + // everything came back on the next scan two seconds later + if (inline.DiscardAll(out var message)) + { + snapshots = []; + } + else + { + Log.Error(message ?? "Could not discard the pending snapshots."); + } } ///