From b22a35e4ad71a42d488a2bda5dd6d6576b212171 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Mon, 20 Jul 2026 08:17:27 -0700 Subject: [PATCH] docs(advanced-search): tighten comments for publishing --- .../Advanced Search/AdvancedSearchGump.cs | 17 ++++------- .../AdvancedSearchThreadWorker.cs | 30 +++++++------------ 2 files changed, 16 insertions(+), 31 deletions(-) diff --git a/Projects/UOContent/Engines/Advanced Search/AdvancedSearchGump.cs b/Projects/UOContent/Engines/Advanced Search/AdvancedSearchGump.cs index 869e98cf7..7f72c02b2 100644 --- a/Projects/UOContent/Engines/Advanced Search/AdvancedSearchGump.cs +++ b/Projects/UOContent/Engines/Advanced Search/AdvancedSearchGump.cs @@ -133,9 +133,7 @@ public class AdvancedSearchGump : Gump public AdvancedSearchGump() : base(50, 50) => Build(); - // Number of result entries that actually fit on the current page, given how many - // remain after DisplayFrom. Prevents the paging loop from reading/rendering past - // the end of SearchResults on a partial last page. + // Entries on the current page — bounds the paging loop so a partial last page can't read past the end. internal static int VisibleCount(int total, int displayFrom, int maxEntries) => Math.Clamp(total - displayFrom, 0, maxEntries); @@ -334,8 +332,7 @@ public class AdvancedSearchGump : Gump var allDisplayedSelected = true; - // Bounded to the entries that actually exist on this page so a partial last - // page (fewer than MaxEntries remaining) still renders in descending mode. + // Bound to this page's real entries so a partial last page still renders in descending mode. var visibleCount = VisibleCount(SearchResults.Length, DisplayFrom, MaxEntries); for (var i = 0; i < visibleCount; i++) @@ -779,9 +776,8 @@ public class AdvancedSearchGump : Gump var type = Filter.FilterType ? Filter.Type : null; - // Push the entities. Workers read entity state concurrently with this thread's own - // subsequent mutations of the world; see the class comment on - // AdvancedSearchThreadWorker for the accepted, bounded race this implies. + // Push the entities. Workers read entity state concurrently with the main loop — + // see AdvancedSearchThreadWorker for the accepted, bounded race. foreach (var item in World.Items.Values) { if (type == null || type.IsInstanceOfType(item)) @@ -833,9 +829,8 @@ public class AdvancedSearchGump : Gump } catch (Exception ex) { - // Never let a drain-phase failure escape unhandled on the ThreadPool - // thread - that would terminate the process. The finally below still - // restores autosave and releases the search guard. + // A drain-phase throw here would terminate the process; the finally still + // restores autosave and releases the guard. _logger.Warning(ex, "AdvancedSearch: search drain failed"); } finally diff --git a/Projects/UOContent/Engines/Advanced Search/AdvancedSearchThreadWorker.cs b/Projects/UOContent/Engines/Advanced Search/AdvancedSearchThreadWorker.cs index 62be513a8..a350cbdcd 100644 --- a/Projects/UOContent/Engines/Advanced Search/AdvancedSearchThreadWorker.cs +++ b/Projects/UOContent/Engines/Advanced Search/AdvancedSearchThreadWorker.cs @@ -10,19 +10,13 @@ using Server.Multis; namespace Server.Engines.AdvancedSearch; /// -/// Runs entity filtering on a dedicated background thread while the main event loop keeps -/// mutating those same entities. This is an intentional, bounded race: workers read live -/// / state (location, map, properties, etc.) without any -/// synchronization with the main thread. Torn reads of multi-field value types like -/// can yield a stale-but-plausible combination of coordinates rather than -/// the true before- or after-mutation value, and any exception thrown by a getter (e.g. from a -/// property being torn down mid-read) is caught and logged per-entity in -/// , which simply skips that entity instead of crashing the worker -/// or the search. Results are therefore a best-effort snapshot that may occasionally omit or -/// misreport an entity that was concurrently modified or deleted, never a page fault or bad -/// server state. Fully eliminating the race would require copying the fields each filter reads -/// onto the main thread before handing entities off to workers; that snapshotting is a larger -/// change and is deferred. +/// Filters entities on a background thread while the main loop keeps mutating them — an +/// intentional, bounded race. Reads of live / state are +/// unsynchronized, so a torn read may report stale coordinates, and any +/// getter that throws mid-read is caught per-entity in and skipped. +/// Results are best-effort and may omit a concurrently modified entity, but never fault or corrupt +/// server state. Eliminating the race would require snapshotting each read field onto the main +/// thread before handing entities off; that is deferred. /// public class AdvancedSearchThreadWorker { @@ -118,9 +112,7 @@ public class AdvancedSearchThreadWorker } else { - // Queue is transiently empty but we're not paused yet; yield the core to - // other runnable threads (e.g. the other still-filtering workers) instead - // of busy-spinning. + // Transiently empty but not yet paused: yield rather than busy-spin. Thread.Yield(); } } @@ -157,8 +149,7 @@ public class AdvancedSearchThreadWorker { if (_filter == null) { - // Exit() clears the filter as part of tearing down the worker; a straggler - // entity dequeued after that point must bail out instead of NREing below. + // Exit() clears the filter; a straggler entity dequeued after teardown bails here. return null; } @@ -193,8 +184,7 @@ public class AdvancedSearchThreadWorker return null; } - // Deferred until after the cheap map/range/region filters so entities that don't - // even qualify for this search skip the expensive house/keyring enumeration below. + // After the cheap filters, so non-qualifying entities skip the house/keyring enumeration. if (_filter.HideValidInternalMap) { HandleValidInternal(entity);