From 65bdfe6cd74a4ad28d68b94b7d0a60903c3cf269 Mon Sep 17 00:00:00 2001
From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com>
Date: Thu, 30 Jul 2026 18:33:32 -0700
Subject: [PATCH] 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>
---
 .../Tests/PropertyList/ObjectPropertyListReentrancyTests.cs  | 4 ++++
 .../Tests/Network/AutoDenylist/AutoDenylistTests.cs          | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/Projects/Server.Tests/Tests/PropertyList/ObjectPropertyListReentrancyTests.cs b/Projects/Server.Tests/Tests/PropertyList/ObjectPropertyListReentrancyTests.cs
index 8da5f748a..b7816ae96 100644
--- a/Projects/Server.Tests/Tests/PropertyList/ObjectPropertyListReentrancyTests.cs
+++ b/Projects/Server.Tests/Tests/PropertyList/ObjectPropertyListReentrancyTests.cs
@@ -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]
diff --git a/Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs b/Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs
index 71aa03aeb..ef5788ee4 100644
--- a/Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs
+++ b/Projects/UOContent.Tests/Tests/Network/AutoDenylist/AutoDenylistTests.cs
@@ -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