docs(advanced-search): tighten comments for publishing

This commit is contained in:
Kamron Batman 2026-07-20 08:17:27 -07:00
parent dcedf91d9e
commit b22a35e4ad
2 changed files with 16 additions and 31 deletions

View file

@ -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

View file

@ -10,19 +10,13 @@ using Server.Multis;
namespace Server.Engines.AdvancedSearch;
/// <summary>
/// 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
/// <see cref="Item"/>/<see cref="Mobile"/> state (location, map, properties, etc.) without any
/// synchronization with the main thread. Torn reads of multi-field value types like
/// <see cref="Point3D"/> 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
/// <see cref="DoEntitySearch"/>, 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 <see cref="Item"/>/<see cref="Mobile"/> state are
/// unsynchronized, so a torn <see cref="Point3D"/> read may report stale coordinates, and any
/// getter that throws mid-read is caught per-entity in <see cref="DoEntitySearch"/> 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.
/// </summary>
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);