## Summary
Hardens the **Advanced Search** engine (`Projects/UOContent/Engines/Advanced Search/`) — the GM entity finder that fans searches across background worker threads. A code review surfaced 14 defects (A–N), including a shard-crasher reachable from a single admin typo and a path that silently disables autosave for the rest of the shard's uptime. Each behavioral fix ships with a test.
Full `UOContent.Tests` suite: **530/530 green** (21 new AdvancedSearch tests).
## Fixes
### Crash / data-loss
- **A — Shard crash on a malformed Property Test.** `AdvancedSearchThreadWorker.Execute` had no `try/catch` and the worker `Thread` is foreground, so a parse throw (`Hits>abc`, `Layer=onehanded` — `Enum.Parse` was case-sensitive, `Hits>1@` — empty sub-expression indexing) terminated the process. Now: `ParseValue`/`CompareValues` use `TryParse`/`Enum.TryParse(ignoreCase)` and return no-match instead of throwing; the per-entity filter is wrapped in `try/catch` (logs + skips); empty expressions are guarded.
- **C — Overlapping searches corrupt state + brick autosave.** `_threadWorkers`/`_threadId` were `static` but `DoSearch` is an instance method; a second search (double-click / two admins) stomped shared worker state and could leave a drain waiting forever on the shared `AutoResetEvent`, so `AutoSave.SavesEnabled` was never restored. Now: an `Interlocked` re-entrancy guard rejects concurrent searches.
- **G — Autosave restore not guaranteed.** The restore lived only in the success callback. Now it's in a `finally` (plus an outer `catch` covering the synchronous setup and a `catch` on the drain body), so autosave + the guard are always released.
### Wrong results
- **D — `@`/`|` operator precedence.** `a@b|c` evaluated as `a && (b || c)` instead of `(a && b) || c`. OR now binds looser than AND (`AdvancedSearchUtilities.EvaluateBoolean`, unit-tested).
- **E — Descending sort, partial last page rendered blank** (the index decreased in descending mode and the `break` early-out killed the loop). Now a bounded `VisibleCount`-driven loop renders the last page in both directions.
- **F — Deleted entities** were not skipped (ghost rows). Now `DoEntitySearch` skips `entity.Deleted`.
- **N — Reference-type comparisons** threw (`Comparer<T>.Default.Compare` on non-`IComparable`) and compared references to a string. Now equality is by value and ordering is guarded to `IComparable` (no throw).
### Worker perf / hardening
- **H** busy-spin → `Thread.Yield()` in the drain; **I** `GetProperties()` cached per `Type`; **J** `HandleValidInternal` moved behind the cheap map/range/region filters; **K** worker threads are `IsBackground` + `Exit()` tolerates an already-terminated worker; **L** `_filter == null` guard; **M** consistent `Volatile` access on `_pause`/`_exit`.
### Documented
- **B** — the residual worker/event-loop read race is documented on `AdvancedSearchThreadWorker`: workers read live entity state concurrently with the loop, so value-type reads may be stale-but-safe and getter exceptions are swallowed; fully eliminating it would require snapshotting entity fields on the main thread (deferred).
## Notes
- New test-only seams (`TryBeginSearch`/`EndSearch`/`IsSearchInProgress`/`VisibleCount`/`TryParseValue`/`EvaluateBoolean`) are `internal` via the existing `InternalsVisibleTo("UOContent.Tests")`.
- Dead `public ParseValue<T>` removed.
- `ConcurrentDictionary` for the reflection cache is intentional — these workers are genuinely parallel.
438 lines
16 KiB
C#
438 lines
16 KiB
C#
using System;
|
|
using System.Buffers;
|
|
using System.Collections.Generic;
|
|
using System.Globalization;
|
|
using System.Numerics;
|
|
using System.Runtime.CompilerServices;
|
|
|
|
namespace Server.Engines.AdvancedSearch;
|
|
|
|
public static class AdvancedSearchUtilities
|
|
{
|
|
private static readonly SearchValues<char> _operators = SearchValues.Create(['=', '!', '>', '<', '~']);
|
|
|
|
public static ReadOnlySpan<char> FindOperatorIndex(ReadOnlySpan<char> expression, out int index)
|
|
{
|
|
index = expression.IndexOfAny(_operators);
|
|
if (index == -1)
|
|
{
|
|
return ReadOnlySpan<char>.Empty;
|
|
}
|
|
|
|
// We are at the end
|
|
if (index + 1 == expression.Length)
|
|
{
|
|
return expression.Slice(index, 1);
|
|
}
|
|
|
|
// Look for double character
|
|
// <=, >=, ~<, ~>, ~~, ~=, ~!
|
|
var op = expression[index];
|
|
var next = expression[index + 1];
|
|
if (next is '=' && op is '=' or '<' or '>' or '~' or '!' || op is '~' && next is '<' or '>' or '~' or '!')
|
|
{
|
|
return expression.Slice(index, 2);
|
|
}
|
|
|
|
return expression.Slice(index, 1);
|
|
}
|
|
|
|
public static bool CompareValues(Type propertyType, object propertyValue, ReadOnlySpan<char> valuePart, ReadOnlySpan<char> operatorSpan)
|
|
{
|
|
// TODO: Add support for implicit conversion types like Serial -> uint
|
|
|
|
if (propertyType == typeof(long))
|
|
{
|
|
return TryParseValue<long>(valuePart, out var parsedValue) &&
|
|
CompareNumeric((long)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(ulong))
|
|
{
|
|
return TryParseValue<ulong>(valuePart, out var parsedValue) &&
|
|
CompareNumeric((ulong)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(int))
|
|
{
|
|
return TryParseValue<int>(valuePart, out var parsedValue) &&
|
|
CompareNumeric((int)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(uint))
|
|
{
|
|
return TryParseValue<uint>(valuePart, out var parsedValue) &&
|
|
CompareNumeric((uint)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(short))
|
|
{
|
|
return TryParseValue<short>(valuePart, out var parsedValue) &&
|
|
CompareNumeric((short)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(ushort))
|
|
{
|
|
return TryParseValue<ushort>(valuePart, out var parsedValue) &&
|
|
CompareNumeric((ushort)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(sbyte))
|
|
{
|
|
return TryParseValue<sbyte>(valuePart, out var parsedValue) &&
|
|
CompareNumeric((sbyte)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(byte))
|
|
{
|
|
return TryParseValue<byte>(valuePart, out var parsedValue) &&
|
|
CompareNumeric((byte)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(float))
|
|
{
|
|
return TryParseValue<float>(valuePart, out var parsedValue) &&
|
|
Compare((float)propertyValue!, parsedValue, valuePart, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(double))
|
|
{
|
|
return TryParseValue<double>(valuePart, out var parsedValue) &&
|
|
Compare((double)propertyValue!, parsedValue, valuePart, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(string))
|
|
{
|
|
return TryParseValue<string>(valuePart, out var parsedValue) &&
|
|
Compare((string)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(TimeSpan))
|
|
{
|
|
return TryParseValue<TimeSpan>(valuePart, out var parsedValue) &&
|
|
Compare((TimeSpan)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(DateTime))
|
|
{
|
|
return TryParseValue<DateTime>(valuePart, out var parsedValue) &&
|
|
Compare((DateTime)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType == typeof(bool))
|
|
{
|
|
return TryParseValue<bool>(valuePart, out var parsedValue) &&
|
|
Compare((bool)propertyValue!, parsedValue, operatorSpan);
|
|
}
|
|
if (propertyType.IsEnum)
|
|
{
|
|
if (!Enum.TryParse(propertyType, valuePart.ToString(), true, out var valueEnum) || valueEnum == null)
|
|
{
|
|
return false;
|
|
}
|
|
|
|
return GetEnumSize(propertyType) switch
|
|
{
|
|
1 => CompareNumeric((byte)propertyValue!, (byte)valueEnum, operatorSpan),
|
|
2 => CompareNumeric((short)propertyValue!, (short)valueEnum, operatorSpan),
|
|
4 => CompareNumeric((int)propertyValue!, (int)valueEnum, operatorSpan),
|
|
8 => CompareNumeric((long)propertyValue!, (long)valueEnum, operatorSpan),
|
|
_ => false
|
|
};
|
|
}
|
|
// Anything the hot typed paths above didn't handle — reference types (Poison, Map, entity
|
|
// properties resolved by serial), IParsable value types (Guid, decimal, ...), and legacy
|
|
// RunUO types with a static Parse(string) (Faction, Town, ...). Delegate to the shared,
|
|
// thread-safe Types converter so the target is parsed into the property's real type, then
|
|
// compare by value. A string is allocated here, but this is the uncommon path; the common
|
|
// types never reach it. Types returns a non-null message when it can't parse -> no match.
|
|
return Types.TryParse(propertyType, valuePart.ToString(), out var parsed) == null &&
|
|
CompareReference(propertyValue!, parsed, operatorSpan);
|
|
}
|
|
|
|
public static bool CompareNumeric<T>(T propertyValue, T parsedValue, ReadOnlySpan<char> operatorSpan) where T : INumber<T> =>
|
|
operatorSpan switch
|
|
{
|
|
"=" or "==" => propertyValue == parsedValue,
|
|
"!" or "!=" => propertyValue != parsedValue,
|
|
">" => propertyValue > parsedValue,
|
|
"<" => propertyValue < parsedValue,
|
|
">=" => propertyValue >= parsedValue,
|
|
"<=" => propertyValue <= parsedValue,
|
|
_ => false
|
|
};
|
|
|
|
public static bool Compare(
|
|
double propertyValue,
|
|
double parsedValue,
|
|
ReadOnlySpan<char> originalValue,
|
|
ReadOnlySpan<char> operatorSpan
|
|
)
|
|
{
|
|
var epsilon = CalculateEpsilon(originalValue);
|
|
|
|
return operatorSpan switch
|
|
{
|
|
"=" or "==" => Math.Abs(propertyValue - parsedValue) < epsilon,
|
|
"!" or "!=" => Math.Abs(propertyValue - parsedValue) >= epsilon,
|
|
">" => propertyValue > parsedValue + epsilon,
|
|
"<" => propertyValue < parsedValue - epsilon,
|
|
">=" => propertyValue >= parsedValue - epsilon,
|
|
"<=" => propertyValue <= parsedValue + epsilon,
|
|
_ => throw new ArgumentException("Invalid operator")
|
|
};
|
|
}
|
|
|
|
public static double CalculateEpsilon(ReadOnlySpan<char> value)
|
|
{
|
|
var decimalPlace = value.IndexOf('.');
|
|
|
|
if (decimalPlace == -1)
|
|
{
|
|
// No decimal point, so use a default small epsilon
|
|
return 1E-10;
|
|
}
|
|
|
|
// Convert decimal places to a negative power of 10
|
|
return (value.Length - decimalPlace - 1) switch
|
|
{
|
|
< 10 => 1E-10,
|
|
10 => 1E-11,
|
|
11 => 1E-12,
|
|
12 => 1E-13,
|
|
13 => 1E-14,
|
|
14 => 1E-15,
|
|
_ => 1E-16
|
|
};
|
|
}
|
|
|
|
public static bool Compare(string propertyValue, string parsedValue, ReadOnlySpan<char> operatorSpan) =>
|
|
operatorSpan switch
|
|
{
|
|
"=" or "==" => propertyValue.EqualsOrdinal(parsedValue),
|
|
"!" or "!=" => !propertyValue.EqualsOrdinal(parsedValue),
|
|
">" => propertyValue.StartsWithOrdinal(parsedValue),
|
|
"<" => propertyValue.EndsWithOrdinal(parsedValue),
|
|
"~" => propertyValue.Contains(parsedValue),
|
|
"~<" => propertyValue.InsensitiveEndsWith(parsedValue),
|
|
"~>" => propertyValue.InsensitiveStartsWith(parsedValue),
|
|
"~~" => propertyValue.InsensitiveContains(parsedValue),
|
|
"~=" => propertyValue.InsensitiveEquals(parsedValue),
|
|
"~!" => !propertyValue.InsensitiveEquals(parsedValue),
|
|
_ => false
|
|
};
|
|
|
|
public static bool Compare(TimeSpan propertyValue, TimeSpan parsedValue, ReadOnlySpan<char> operatorSpan) =>
|
|
operatorSpan switch
|
|
{
|
|
"=" or "==" => propertyValue == parsedValue,
|
|
"!" or "!=" => propertyValue != parsedValue,
|
|
">" => propertyValue > parsedValue,
|
|
"<" => propertyValue < parsedValue,
|
|
">=" => propertyValue >= parsedValue,
|
|
"<=" => propertyValue <= parsedValue,
|
|
_ => false
|
|
};
|
|
|
|
public static bool Compare(DateTime propertyValue, DateTime parsedValue, ReadOnlySpan<char> operatorSpan) =>
|
|
operatorSpan switch
|
|
{
|
|
"=" or "==" => propertyValue == parsedValue,
|
|
"!" or "!=" => propertyValue != parsedValue,
|
|
">" => propertyValue > parsedValue,
|
|
"<" => propertyValue < parsedValue,
|
|
">=" => propertyValue >= parsedValue,
|
|
"<=" => propertyValue <= parsedValue,
|
|
_ => false
|
|
};
|
|
|
|
public static bool Compare(bool propertyValue, bool parsedValue, ReadOnlySpan<char> operatorSpan) =>
|
|
operatorSpan switch
|
|
{
|
|
"=" or "==" => propertyValue == parsedValue,
|
|
"!" or "!=" => propertyValue != parsedValue,
|
|
_ => false
|
|
};
|
|
|
|
public static bool CompareReference<T>(T propertyValue, T parsedValue, ReadOnlySpan<char> operatorSpan)
|
|
{
|
|
switch (operatorSpan)
|
|
{
|
|
case "=":
|
|
case "==": return Equals(propertyValue, parsedValue);
|
|
case "!":
|
|
case "!=": return !Equals(propertyValue, parsedValue);
|
|
}
|
|
|
|
if (propertyValue is IComparable cmp && parsedValue != null)
|
|
{
|
|
try
|
|
{
|
|
var c = cmp.CompareTo(parsedValue);
|
|
return operatorSpan switch
|
|
{
|
|
">" => c > 0,
|
|
"<" => c < 0,
|
|
">=" => c >= 0,
|
|
"<=" => c <= 0,
|
|
_ => false
|
|
};
|
|
}
|
|
catch
|
|
{
|
|
return false;
|
|
}
|
|
}
|
|
|
|
return false;
|
|
}
|
|
|
|
internal static bool TryParseValue<T>(ReadOnlySpan<char> valuePart, out T value)
|
|
{
|
|
// Special handling for boolean and hexadecimal values
|
|
if (typeof(T) == typeof(bool))
|
|
{
|
|
var val = valuePart.ToString().ToLower();
|
|
if (val is "true" or "1" or "enabled" or "on")
|
|
{
|
|
value = (T)(object)true;
|
|
return true;
|
|
}
|
|
|
|
if (val is "false" or "0" or "disabled" or "off")
|
|
{
|
|
value = (T)(object)false;
|
|
return true;
|
|
}
|
|
|
|
value = default;
|
|
return false;
|
|
}
|
|
|
|
if (typeof(T) == typeof(long))
|
|
{
|
|
return TryParseNumericValue<long, T>(valuePart, out value);
|
|
}
|
|
|
|
if (typeof(T) == typeof(ulong))
|
|
{
|
|
return TryParseNumericValue<ulong, T>(valuePart, out value);
|
|
}
|
|
|
|
if (typeof(T) == typeof(int))
|
|
{
|
|
return TryParseNumericValue<int, T>(valuePart, out value);
|
|
}
|
|
|
|
if (typeof(T) == typeof(uint))
|
|
{
|
|
return TryParseNumericValue<uint, T>(valuePart, out value);
|
|
}
|
|
|
|
if (typeof(T) == typeof(short))
|
|
{
|
|
return TryParseNumericValue<short, T>(valuePart, out value);
|
|
}
|
|
|
|
if (typeof(T) == typeof(ushort))
|
|
{
|
|
return TryParseNumericValue<ushort, T>(valuePart, out value);
|
|
}
|
|
|
|
if (typeof(T) == typeof(sbyte))
|
|
{
|
|
return TryParseNumericValue<sbyte, T>(valuePart, out value);
|
|
}
|
|
|
|
if (typeof(T) == typeof(byte))
|
|
{
|
|
return TryParseNumericValue<byte, T>(valuePart, out value);
|
|
}
|
|
|
|
if (typeof(T) == typeof(float))
|
|
{
|
|
return TryParseNumericValue<float, T>(valuePart, out value);
|
|
}
|
|
|
|
if (typeof(T) == typeof(double))
|
|
{
|
|
return TryParseNumericValue<double, T>(valuePart, out value);
|
|
}
|
|
|
|
// string needs no parsing — the span itself is the value.
|
|
if (typeof(T) == typeof(string))
|
|
{
|
|
value = (T)(object)valuePart.ToString();
|
|
return true;
|
|
}
|
|
|
|
// Remaining supported types (TimeSpan, DateTime) parse straight from the span via
|
|
// ISpanParsable<T> — no allocation, no reflection, and unlike Convert.ChangeType it handles
|
|
// TimeSpan, which is not IConvertible and previously failed silently.
|
|
if (typeof(T) == typeof(TimeSpan))
|
|
{
|
|
return TryParseSpanParsable<TimeSpan, T>(valuePart, out value);
|
|
}
|
|
|
|
if (typeof(T) == typeof(DateTime))
|
|
{
|
|
return TryParseSpanParsable<DateTime, T>(valuePart, out value);
|
|
}
|
|
|
|
value = default;
|
|
return false;
|
|
}
|
|
|
|
// Parses U (a value type exposing ISpanParsable<U>) from the span and reinterprets it as T. The
|
|
// two type params mirror TryParseNumericValue: the caller dispatches on typeof(T), so U == T at
|
|
// every call site and the (T)(object) cast is always valid.
|
|
[MethodImpl(MethodImplOptions.AggressiveInlining)]
|
|
private static bool TryParseSpanParsable<U, T>(ReadOnlySpan<char> valuePart, out T value) where U : ISpanParsable<U>
|
|
{
|
|
if (U.TryParse(valuePart, null, out var parsed))
|
|
{
|
|
value = (T)(object)parsed;
|
|
return true;
|
|
}
|
|
|
|
value = default;
|
|
return false;
|
|
}
|
|
|
|
[MethodImpl(MethodImplOptions.AggressiveInlining)]
|
|
private static bool TryParseNumericValue<T, R>(ReadOnlySpan<char> valuePart, out R value) where T : INumber<T>
|
|
{
|
|
var ok = valuePart.StartsWith("0x")
|
|
? T.TryParse(valuePart[2..], NumberStyles.HexNumber, null, out var parsed)
|
|
: T.TryParse(valuePart, null, out parsed);
|
|
|
|
if (ok)
|
|
{
|
|
value = (R)(object)parsed;
|
|
return true;
|
|
}
|
|
|
|
value = default;
|
|
return false;
|
|
}
|
|
|
|
// Evaluates one trimmed leaf atom against caller-supplied state. A custom delegate is required
|
|
// because ReadOnlySpan<char> cannot be a Func<> type argument; passing state avoids a per-call
|
|
// capturing closure, so the recursion allocates neither a string nor a closure.
|
|
internal delegate bool LeafEvaluator<in TState>(TState state, ReadOnlySpan<char> leaf);
|
|
|
|
// OR ('|') binds looser than AND ('@'); split on the outermost OR first, then AND.
|
|
internal static bool EvaluateBoolean<TState>(ReadOnlySpan<char> expr, TState state, LeafEvaluator<TState> evalLeaf)
|
|
{
|
|
var orIndex = expr.IndexOf('|');
|
|
if (orIndex != -1)
|
|
{
|
|
return EvaluateBoolean(expr[..orIndex], state, evalLeaf) || EvaluateBoolean(expr[(orIndex + 1)..], state, evalLeaf);
|
|
}
|
|
|
|
var andIndex = expr.IndexOf('@');
|
|
if (andIndex != -1)
|
|
{
|
|
return EvaluateBoolean(expr[..andIndex], state, evalLeaf) && EvaluateBoolean(expr[(andIndex + 1)..], state, evalLeaf);
|
|
}
|
|
|
|
return evalLeaf(state, expr.Trim());
|
|
}
|
|
|
|
private static int GetEnumSize(Type enumType) =>
|
|
Type.GetTypeCode(Enum.GetUnderlyingType(enumType)) switch
|
|
{
|
|
TypeCode.Byte or TypeCode.SByte => sizeof(byte),
|
|
TypeCode.Int16 or TypeCode.UInt16 => sizeof(ushort),
|
|
TypeCode.Int32 or TypeCode.UInt32 => sizeof(uint),
|
|
TypeCode.Int64 or TypeCode.UInt64 => sizeof(ulong),
|
|
_ => 4
|
|
};
|
|
}
|