fix(tests): serialize test classes that rent through STArrayPool

STArrayPool is single-threaded by design and its bucket cache is a plain static,
not thread-static, with a check-then-act initialize in Return():

    var cacheBuckets = _cacheBuckets ?? InitializeBuckets();

Two threads both see null, both initialize, and the loser trips
Debug.Assert(_cacheBuckets is null). Anything renting from it therefore has to be
kept off parallel test threads, which is what the DisableParallelization
collections exist for.

ObjectPropertyListReentrancyTests and ObjectPropertyListNestedBuildTests build
property lists, which rent the interpolation buffer, but were not in the
sequential collection -- unlike PropertyListInvalidationDuringBuildTests in the
same file. That is the macOS CI failure on this PR: Debug.Fail "Non-null
_cacheBuckets" out of ObjectPropertyList.Dispose. It is timing-dependent, so it
surfaces on some platforms and not others.

AutoDenylistTests has the same exposure: its cap tests reach AutoDenylist.Sweep,
which rents a PooledRefList without mt. The blocklist tests need no marking
because BlocklistSnapshot.Build asks for the mt pool explicitly, which is the
distinction worth remembering.

No production change: STArrayPool is the right pool on the game loop, where both
Sweep and the property list actually run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Kamron Batman 2026-07-30 18:33:32 -07:00
parent 927c87702d
commit 65bdfe6cd7
No known key found for this signature in database
GPG key ID: 7D81DF26D9A5D94A
2 changed files with 9 additions and 0 deletions

View file

@ -8,6 +8,9 @@ namespace Server.Tests;
/// hole is evaluated while it is live. A Reset()/Dispose() landing in that window used to leave the
/// next Append* spanning a null array: ArgumentNullException, parameter "array".
/// </summary>
// Sequential: building a list rents and returns through STArrayPool, whose bucket cache is a plain static
// with a check-then-act initialize. Two test classes doing this on different threads race it.
[Collection("Sequential Server Tests")]
public class ObjectPropertyListReentrancyTests
{
// Stands in for a property getter that invalidates while its own tooltip is being built.
@ -52,6 +55,7 @@ public class ObjectPropertyListReentrancyTests
/// The guard is per-list, so nested builds (a GetProperties override that reads another entity's
/// PropertyList) cannot unguard the outer one the way a single shared slot would.
/// </summary>
[Collection("Sequential Server Tests")] // same STArrayPool exposure as above
public class ObjectPropertyListNestedBuildTests
{
[Fact]

View file

@ -22,6 +22,11 @@ namespace Server.Tests.Network.AutoDenylists;
// Static store, so every test resets it first. Addresses come from TEST-NET-2 (198.51.100.0/24) to stay clear
// of the other ban tests if xUnit runs these classes concurrently.
//
// Sequential because the cap tests reach AutoDenylist.Sweep, which rents through STArrayPool. That pool is
// single-threaded by design and its bucket cache is a plain static, so touching it from two test threads
// races. The blocklist tests need no such marking because BlocklistSnapshot.Build asks for the mt pool.
[Collection("Sequential UOContent Tests")]
public class AutoDenylistTests
{
private const long DurationMs = 900_000; // 15 minutes, the shipped default