From f8c594779e07ca6c2d349f06b1cb8e2641463d7c Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sun, 19 Jul 2026 21:53:29 -0700 Subject: [PATCH] fix(advanced-search): C/G follow-up - cover synchronous setup + test isolation in try/finally --- .../AdvancedSearchWorkerTests.cs | 15 ++- .../Advanced Search/AdvancedSearchGump.cs | 125 +++++++++--------- 2 files changed, 78 insertions(+), 62 deletions(-) diff --git a/Projects/UOContent.Tests/Tests/Engines/AdvancedSearch/AdvancedSearchWorkerTests.cs b/Projects/UOContent.Tests/Tests/Engines/AdvancedSearch/AdvancedSearchWorkerTests.cs index 36f392296..2bfe94af8 100644 --- a/Projects/UOContent.Tests/Tests/Engines/AdvancedSearch/AdvancedSearchWorkerTests.cs +++ b/Projects/UOContent.Tests/Tests/Engines/AdvancedSearch/AdvancedSearchWorkerTests.cs @@ -77,10 +77,19 @@ public class AdvancedSearchWorkerTests public void DoSearch_IsGuarded_AgainstReentry() { // White-box: flip the guard, assert a second entry is rejected, then clear. + // _searchInProgress is process-global static state; release it in finally so a + // failed assert here can't leak the guard into other tests. Assert.False(AdvancedSearchGump.IsSearchInProgress); - Assert.True(AdvancedSearchGump.TryBeginSearch()); // acquires - Assert.False(AdvancedSearchGump.TryBeginSearch()); // rejected - AdvancedSearchGump.EndSearch(); // releases + Assert.True(AdvancedSearchGump.TryBeginSearch()); // acquires + try + { + Assert.False(AdvancedSearchGump.TryBeginSearch()); // rejected + } + finally + { + AdvancedSearchGump.EndSearch(); // releases + } + Assert.False(AdvancedSearchGump.IsSearchInProgress); } } diff --git a/Projects/UOContent/Engines/Advanced Search/AdvancedSearchGump.cs b/Projects/UOContent/Engines/Advanced Search/AdvancedSearchGump.cs index 64c7ec950..8d4222aaf 100644 --- a/Projects/UOContent/Engines/Advanced Search/AdvancedSearchGump.cs +++ b/Projects/UOContent/Engines/Advanced Search/AdvancedSearchGump.cs @@ -754,80 +754,87 @@ public class AdvancedSearchGump : Gump _threadId = 0; var autoSave = AutoSave.SavesEnabled; - if (autoSave) + AutoSave.SavesEnabled = false; + + try { - AutoSave.SavesEnabled = false; - } + _threadWorkers ??= new AdvancedSearchThreadWorker[Math.Max(Environment.ProcessorCount - 1, 1)]; - _threadWorkers ??= new AdvancedSearchThreadWorker[Math.Max(Environment.ProcessorCount - 1, 1)]; + var ignoreQueue = new ConcurrentQueue(); + var results = new ConcurrentQueue(); + var worldLocation = new WorldLocation(from.Location, from.Map); - var ignoreQueue = new ConcurrentQueue(); - var results = new ConcurrentQueue(); - var worldLocation = new WorldLocation(from.Location, from.Map); - - for (var i = 0; i < _threadWorkers.Length; i++) - { - (_threadWorkers[i] ??= new AdvancedSearchThreadWorker()).Wake(worldLocation, Filter, results, ignoreQueue); - } - - var type = Filter.FilterType ? Filter.Type : null; - - // Push the entities - foreach (var item in World.Items.Values) - { - if (type == null || type.IsInstanceOfType(item)) + for (var i = 0; i < _threadWorkers.Length; i++) { - PushToWorkers(item); + (_threadWorkers[i] ??= new AdvancedSearchThreadWorker()).Wake(worldLocation, Filter, results, ignoreQueue); } - } - foreach (var m in World.Mobiles.Values) - { - if (type == null || type.IsInstanceOfType(m)) - { - PushToWorkers(m); - } - } + var type = Filter.FilterType ? Filter.Type : null; - ThreadPool.QueueUserWorkItem(state => - { - try + // Push the entities + foreach (var item in World.Items.Values) { - // Block until everything is processed - for (var i = 0; i < _threadWorkers.Length; i++) + if (type == null || type.IsInstanceOfType(item)) { - _threadWorkers[i].Sleep(); + PushToWorkers(item); } + } - var ignoredEntities = new HashSet(ignoreQueue); - - // Force the GC to collect the ignored entities - ignoreQueue.Clear(); - - var resultsList = new List(results.Count); - foreach (var result in results) + foreach (var m in World.Mobiles.Values) + { + if (type == null || type.IsInstanceOfType(m)) { - if (!ignoredEntities.Contains(result.Entity)) + PushToWorkers(m); + } + } + + ThreadPool.QueueUserWorkItem(state => + { + try + { + // Block until everything is processed + for (var i = 0; i < _threadWorkers.Length; i++) { - resultsList.Add(result); + _threadWorkers[i].Sleep(); } + + var ignoredEntities = new HashSet(ignoreQueue); + + // Force the GC to collect the ignored entities + ignoreQueue.Clear(); + + var resultsList = new List(results.Count); + foreach (var result in results) + { + if (!ignoredEntities.Contains(result.Entity)) + { + resultsList.Add(result); + } + } + + SearchResults = resultsList.ToArray(); + + // Force the GC to collect the results + resultsList.Clear(); + resultsList.TrimExcess(); + + // Send the gump on the main thread + Core.LoopContext.Post(() => Resend(from)); } - - SearchResults = resultsList.ToArray(); - - // Force the GC to collect the results - resultsList.Clear(); - resultsList.TrimExcess(); - - // Send the gump on the main thread - Core.LoopContext.Post(() => Resend(from)); - } - finally - { - AutoSave.SavesEnabled = (bool)state!; - EndSearch(); - } - }, autoSave); + finally + { + AutoSave.SavesEnabled = (bool)state!; + EndSearch(); + } + }, autoSave); + } + catch + { + // Setup failed before the work item took ownership of the release. + AutoSave.SavesEnabled = autoSave; + EndSearch(); + throw; + } } private void SetSortSwitches(int radioSwitch)