fix(tests): single shared bootstrap; idempotent SerializationThreadWorker.Exit (#2473)
## Problem Running the full `UOContent.Tests` suite, the test host **hangs ~2.5 minutes at shutdown and then crashes** (`Test host process crashed` / run aborted). The tests themselves are fine — they complete in ~1s — but the process can't exit. Captured via `--blame-hang` dump. The blocking thread: ``` System.Threading.WaitHandle.WaitOne() Server.SerializationThreadWorker.Sleep() SerializationThreadWorker.cs:54 (_stopEvent.WaitOne()) Server.SerializationThreadWorker.Exit() SerializationThreadWorker.cs:61 Server.World.ExitSerializationThreads() World.cs:429 Server.Tests.UOContentFixture..ctor() ``` ### Root cause Both collection fixtures (`UOContentFixture` and `PathfindingTestFixture`) each run the **full process-global ModernUO bootstrap**. `World.Load()` is guarded to run once per process, so the **second** fixture's `World.Load()` is a no-op and does **not** respawn the serialization workers — but `World.ExitSerializationThreads()` is **not** guarded, so the second fixture calls `Exit()` on workers whose threads have already terminated. `Exit()` → `Sleep()` → `_stopEvent.WaitOne()` then blocks forever (a dead thread never sets the event). The first collection's tests run; the second collection's fixture deadlocks in its constructor; the host eventually gets killed. This is why single-collection (filtered) runs were fine — only one fixture ever bootstraps — but the full suite hangs. It's not a parallelization race: even strictly sequential, the second fixture deadlocks. ## Fix **(a) Engine — idempotent `SerializationThreadWorker.Exit()`** A second `Exit()` is now a safe no-op instead of a permanent block. Only the owning (main) thread calls `Exit()`, so no synchronization is needed, and the single-call production shutdown path is unchanged. **(b) Tests — one shared bootstrap, strictly sequential collections** - New `TestServerBootstrap.EnsureInitialized()` runs the superset global init **exactly once per process** (lock + once-flag). - `UOContentFixture` / `PathfindingTestFixture` slim down to delegate to it and no longer tear down global state (which the single-bootstrap model owns for the host's lifetime). - `[assembly: CollectionBehavior(DisableTestParallelization = true)]` so collections never overlap. ## Result | | Before | After | |---|---|---| | Tests run (full suite) | 258 (UOContent collection deadlocked) | **418** | | Outcome | 2.5-min hang → host crash | **418 passed, clean exit** | | Wall time | killed | **~7s** |
This commit is contained in:
parent
1c1fd4d930
commit
412a71dfe0
6 changed files with 139 additions and 153 deletions
|
|
@ -1,4 +1,3 @@
|
||||||
using System;
|
|
||||||
using Xunit;
|
using Xunit;
|
||||||
|
|
||||||
namespace Server.Tests;
|
namespace Server.Tests;
|
||||||
|
|
@ -8,7 +7,7 @@ namespace Server.Tests;
|
||||||
/// Configure client path via MODERNUO_CLIENT_PATH environment variable or place files at C:\Ultima Online Classic.
|
/// Configure client path via MODERNUO_CLIENT_PATH environment variable or place files at C:\Ultima Online Classic.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
[CollectionDefinition("Sequential Server Tests", DisableParallelization = true)]
|
[CollectionDefinition("Sequential Server Tests", DisableParallelization = true)]
|
||||||
public class ServerFixture : ICollectionFixture<ServerFixture>, IDisposable
|
public class ServerFixture : ICollectionFixture<ServerFixture>
|
||||||
{
|
{
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// True if TileData was successfully loaded from client files.
|
/// True if TileData was successfully loaded from client files.
|
||||||
|
|
@ -35,13 +34,8 @@ public class ServerFixture : ICollectionFixture<ServerFixture>, IDisposable
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public static ushort SurfaceImpassableTileId => TestServerInitializer.SurfaceImpassableTileId;
|
public static ushort SurfaceImpassableTileId => TestServerInitializer.SurfaceImpassableTileId;
|
||||||
|
|
||||||
public ServerFixture()
|
// Global init runs exactly once via the shared, guarded TestServerInitializer. The single
|
||||||
{
|
// bootstrap owns global state for the lifetime of the test host, so there is no
|
||||||
TestServerInitializer.Initialize(loadTileData: true);
|
// per-collection teardown — matching UOContent.Tests' TestServerInitializer pattern.
|
||||||
}
|
public ServerFixture() => TestServerInitializer.Initialize(loadTileData: true);
|
||||||
|
|
||||||
public void Dispose()
|
|
||||||
{
|
|
||||||
Timer.Init(0);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -10,7 +10,7 @@ namespace Server.Tests;
|
||||||
/// Shared server initialization logic for test fixtures.
|
/// Shared server initialization logic for test fixtures.
|
||||||
/// Ensures the server is only initialized once across all test collections.
|
/// Ensures the server is only initialized once across all test collections.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public static class TestServerInitializer
|
internal static class TestServerInitializer
|
||||||
{
|
{
|
||||||
private const string DefaultDataDirectory = @"C:\Ultima Online Classic";
|
private const string DefaultDataDirectory = @"C:\Ultima Online Classic";
|
||||||
private static bool _initialized;
|
private static bool _initialized;
|
||||||
|
|
|
||||||
|
|
@ -29,6 +29,7 @@ public class SerializationThreadWorker
|
||||||
private readonly AutoResetEvent _stopEvent; // Main thread waits for the worker finish draining
|
private readonly AutoResetEvent _stopEvent; // Main thread waits for the worker finish draining
|
||||||
private bool _pause;
|
private bool _pause;
|
||||||
private bool _exit;
|
private bool _exit;
|
||||||
|
private bool _exited;
|
||||||
private byte[] _heap;
|
private byte[] _heap;
|
||||||
|
|
||||||
private readonly ConcurrentQueue<IGenericSerializable> _entities;
|
private readonly ConcurrentQueue<IGenericSerializable> _entities;
|
||||||
|
|
@ -56,6 +57,12 @@ public class SerializationThreadWorker
|
||||||
|
|
||||||
public void Exit()
|
public void Exit()
|
||||||
{
|
{
|
||||||
|
if (_exited)
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
_exited = true;
|
||||||
_exit = true;
|
_exit = true;
|
||||||
Wake();
|
Wake();
|
||||||
Sleep();
|
Sleep();
|
||||||
|
|
|
||||||
|
|
@ -1,92 +1,12 @@
|
||||||
using System;
|
|
||||||
using System.IO;
|
|
||||||
using System.Reflection;
|
|
||||||
using Server.Items;
|
|
||||||
using Server.Misc;
|
|
||||||
using Server.Movement;
|
|
||||||
using Server.Tests.Maps;
|
|
||||||
using Xunit;
|
using Xunit;
|
||||||
|
|
||||||
namespace Server.Tests.Pathfinding;
|
namespace Server.Tests.Pathfinding;
|
||||||
|
|
||||||
[CollectionDefinition("Sequential Pathfinding Tests", DisableParallelization = true)]
|
[CollectionDefinition("Sequential Pathfinding Tests", DisableParallelization = true)]
|
||||||
public class PathfindingTestFixture : ICollectionFixture<PathfindingTestFixture>, IDisposable
|
public class PathfindingTestFixture : ICollectionFixture<PathfindingTestFixture>
|
||||||
{
|
{
|
||||||
public PathfindingTestFixture()
|
// Shares the single process-wide bootstrap (TestServerInitializer, in the parent
|
||||||
{
|
// Server.Tests namespace). The superset bootstrap already loads the UO client tile data
|
||||||
Core.ApplicationAssembly = Assembly.GetExecutingAssembly();
|
// and configures movement/pathfinding, so this collection needs no extra setup.
|
||||||
Core.LoopContext = new EventLoopContext();
|
public PathfindingTestFixture() => TestServerInitializer.Initialize();
|
||||||
Core.Expansion = Expansion.EJ;
|
|
||||||
|
|
||||||
ServerConfiguration.Load(true);
|
|
||||||
ServerConfiguration.AssemblyDirectories.Add(Core.BaseDirectory);
|
|
||||||
|
|
||||||
var clientFiles = Environment.GetEnvironmentVariable("MODERNUO_TEST_DATA_DIR")
|
|
||||||
?? @"C:\Ultima Online Classic";
|
|
||||||
ServerConfiguration.DataDirectories.Add(clientFiles);
|
|
||||||
|
|
||||||
AssemblyHandler.LoadAssemblies(["Server.dll", "UOContent.dll"]);
|
|
||||||
|
|
||||||
SkillsInfo.Configure();
|
|
||||||
Server.Network.NetState.Configure();
|
|
||||||
TestMapDefinitions.ConfigureTestMapDefinitions();
|
|
||||||
|
|
||||||
World.Configure();
|
|
||||||
Timer.Init(0);
|
|
||||||
RaceDefinitions.Configure();
|
|
||||||
MovementImpl.Configure();
|
|
||||||
PathFollower.Configure();
|
|
||||||
World.Load();
|
|
||||||
World.ExitSerializationThreads();
|
|
||||||
DecayScheduler.Configure();
|
|
||||||
|
|
||||||
// TileData's static cctor short-circuits when running under xUnit
|
|
||||||
// (see Server/TileData.cs:295). Force-load via reflection so LandTable/ItemTable
|
|
||||||
// flags are populated; without this, every tile reads as flag=None and
|
|
||||||
// MovementImpl.CheckMovement treats everything as walkable.
|
|
||||||
ForceLoadTileData();
|
|
||||||
|
|
||||||
VerifyTrammelTileDataLoaded();
|
|
||||||
}
|
|
||||||
|
|
||||||
private static void ForceLoadTileData()
|
|
||||||
{
|
|
||||||
var loadMethod = typeof(TileData).GetMethod(
|
|
||||||
"Load",
|
|
||||||
BindingFlags.Static | BindingFlags.NonPublic
|
|
||||||
);
|
|
||||||
if (loadMethod == null)
|
|
||||||
{
|
|
||||||
throw new InvalidOperationException(
|
|
||||||
"TileData.Load not found via reflection — engine may have refactored."
|
|
||||||
);
|
|
||||||
}
|
|
||||||
loadMethod.Invoke(null, null);
|
|
||||||
}
|
|
||||||
|
|
||||||
private static void VerifyTrammelTileDataLoaded()
|
|
||||||
{
|
|
||||||
var trammel = Map.Maps[1];
|
|
||||||
if (trammel == null)
|
|
||||||
{
|
|
||||||
throw new InvalidOperationException(
|
|
||||||
"Trammel (mapId=1) was not registered. Check TestMapDefinitions."
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
var tile = trammel.Tiles.GetLandTile(1500, 1600);
|
|
||||||
if (tile.ID == 0)
|
|
||||||
{
|
|
||||||
throw new InvalidOperationException(
|
|
||||||
$"Trammel tile data did not load — GetLandTile(1500,1600) returned ID 0. " +
|
|
||||||
$"Verify Distribution/Data/map1*.mul (or map1LegacyMUL.uop) is present at " +
|
|
||||||
$"{Path.Combine(Core.BaseDirectory, "Data")}."
|
|
||||||
);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
public void Dispose()
|
|
||||||
{
|
|
||||||
Timer.Init(0);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
115
Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs
Normal file
115
Projects/UOContent.Tests/Fixtures/TestServerInitializer.cs
Normal file
|
|
@ -0,0 +1,115 @@
|
||||||
|
using System;
|
||||||
|
using System.IO;
|
||||||
|
using System.Reflection;
|
||||||
|
using System.Threading;
|
||||||
|
using Server.Items;
|
||||||
|
using Server.Misc;
|
||||||
|
using Server.Movement;
|
||||||
|
using Server.Tests.Maps;
|
||||||
|
|
||||||
|
namespace Server.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Single, process-wide ModernUO bootstrap for the UOContent test host. Mirrors Server.Tests'
|
||||||
|
/// TestServerInitializer in name and shape; kept as a separate (non-shared) copy because this
|
||||||
|
/// one loads the UOContent assembly and configures the UOContent-specific systems. Both types
|
||||||
|
/// are <c>internal</c> so the shared name stays scoped to each assembly.
|
||||||
|
///
|
||||||
|
/// ModernUO bootstraps its global singletons (Core, ServerConfiguration, AssemblyHandler,
|
||||||
|
/// NetState/io-ring, World, Timer, the serialization workers, and TileData) exactly once per
|
||||||
|
/// process. <see cref="World.Load"/> is guarded to run once, and
|
||||||
|
/// <see cref="World.ExitSerializationThreads"/> must run once against the live workers. Each
|
||||||
|
/// xUnit collection gets its own fixture instance, so this guard makes the bootstrap run a
|
||||||
|
/// single time regardless of how many collection fixtures are constructed. The two stateful
|
||||||
|
/// collections use <c>[CollectionDefinition(DisableParallelization = true)]</c> so they never
|
||||||
|
/// overlap; pure tests still run in parallel.
|
||||||
|
/// </summary>
|
||||||
|
internal static class TestServerInitializer
|
||||||
|
{
|
||||||
|
private static bool _initialized;
|
||||||
|
private static readonly Lock _lock = new();
|
||||||
|
|
||||||
|
public static void Initialize()
|
||||||
|
{
|
||||||
|
lock (_lock)
|
||||||
|
{
|
||||||
|
if (_initialized)
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
Core.ApplicationAssembly = Assembly.GetExecutingAssembly();
|
||||||
|
Core.LoopContext = new EventLoopContext();
|
||||||
|
Core.Expansion = Expansion.EJ;
|
||||||
|
|
||||||
|
ServerConfiguration.Load(true);
|
||||||
|
ServerConfiguration.AssemblyDirectories.Add(Core.BaseDirectory);
|
||||||
|
|
||||||
|
// Required for the pathfinding tests (real .mul tile data). Harmless for the rest.
|
||||||
|
var clientFiles = Environment.GetEnvironmentVariable("MODERNUO_TEST_DATA_DIR")
|
||||||
|
?? @"C:\Ultima Online Classic";
|
||||||
|
ServerConfiguration.DataDirectories.Add(clientFiles);
|
||||||
|
|
||||||
|
AssemblyHandler.LoadAssemblies(["Server.dll", "UOContent.dll"]);
|
||||||
|
|
||||||
|
SkillsInfo.Configure();
|
||||||
|
Server.Network.NetState.Configure();
|
||||||
|
TestMapDefinitions.ConfigureTestMapDefinitions();
|
||||||
|
|
||||||
|
World.Configure();
|
||||||
|
Timer.Init(0);
|
||||||
|
RaceDefinitions.Configure();
|
||||||
|
MovementImpl.Configure();
|
||||||
|
PathFollower.Configure();
|
||||||
|
World.Load();
|
||||||
|
World.ExitSerializationThreads();
|
||||||
|
DecayScheduler.Configure();
|
||||||
|
|
||||||
|
// TileData's static cctor short-circuits when running under xUnit
|
||||||
|
// (see Server/TileData.cs:295). Force-load via reflection so LandTable/ItemTable
|
||||||
|
// flags are populated; without this, every tile reads as flag=None and
|
||||||
|
// MovementImpl.CheckMovement treats everything as walkable.
|
||||||
|
ForceLoadTileData();
|
||||||
|
|
||||||
|
VerifyTrammelTileDataLoaded();
|
||||||
|
|
||||||
|
_initialized = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private static void ForceLoadTileData()
|
||||||
|
{
|
||||||
|
var loadMethod = typeof(TileData).GetMethod(
|
||||||
|
"Load",
|
||||||
|
BindingFlags.Static | BindingFlags.NonPublic
|
||||||
|
);
|
||||||
|
if (loadMethod == null)
|
||||||
|
{
|
||||||
|
throw new InvalidOperationException(
|
||||||
|
"TileData.Load not found via reflection — engine may have refactored."
|
||||||
|
);
|
||||||
|
}
|
||||||
|
loadMethod.Invoke(null, null);
|
||||||
|
}
|
||||||
|
|
||||||
|
private static void VerifyTrammelTileDataLoaded()
|
||||||
|
{
|
||||||
|
var trammel = Map.Maps[1];
|
||||||
|
if (trammel == null)
|
||||||
|
{
|
||||||
|
throw new InvalidOperationException(
|
||||||
|
"Trammel (mapId=1) was not registered. Check TestMapDefinitions."
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
var tile = trammel.Tiles.GetLandTile(1500, 1600);
|
||||||
|
if (tile.ID == 0)
|
||||||
|
{
|
||||||
|
throw new InvalidOperationException(
|
||||||
|
$"Trammel tile data did not load — GetLandTile(1500,1600) returned ID 0. " +
|
||||||
|
$"Verify Distribution/Data/map1*.mul (or map1LegacyMUL.uop) is present at " +
|
||||||
|
$"{Path.Combine(Core.BaseDirectory, "Data")}."
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -1,63 +1,13 @@
|
||||||
using System;
|
|
||||||
using System.Reflection;
|
|
||||||
using Server.Items;
|
|
||||||
using Server.Misc;
|
|
||||||
using Server.Tests.Maps;
|
|
||||||
using Xunit;
|
using Xunit;
|
||||||
|
|
||||||
namespace Server.Tests;
|
namespace Server.Tests;
|
||||||
|
|
||||||
[CollectionDefinition("Sequential UOContent Tests", DisableParallelization = true)]
|
[CollectionDefinition("Sequential UOContent Tests", DisableParallelization = true)]
|
||||||
public class UOContentFixture : ICollectionFixture<UOContentFixture>, IDisposable
|
public class UOContentFixture : ICollectionFixture<UOContentFixture>
|
||||||
{
|
{
|
||||||
public UOContentFixture()
|
// All process-global initialization lives in TestServerInitializer and runs exactly once,
|
||||||
{
|
// shared across every collection fixture. Tearing down global state here is intentionally
|
||||||
Core.ApplicationAssembly = Assembly.GetExecutingAssembly();
|
// omitted: the world/serialization workers are initialized once and reused for the whole
|
||||||
Core.LoopContext = new EventLoopContext();
|
// test host, so there is nothing per-collection to dispose.
|
||||||
Core.Expansion = Expansion.EJ;
|
public UOContentFixture() => TestServerInitializer.Initialize();
|
||||||
|
|
||||||
// Load Configurations
|
|
||||||
ServerConfiguration.Load(true);
|
|
||||||
|
|
||||||
// Load UOContent.dll into the type resolver
|
|
||||||
ServerConfiguration.AssemblyDirectories.Add(Core.BaseDirectory);
|
|
||||||
AssemblyHandler.LoadAssemblies(["Server.dll", "UOContent.dll"]);
|
|
||||||
|
|
||||||
// Load Skills
|
|
||||||
SkillsInfo.Configure();
|
|
||||||
|
|
||||||
// Configure networking (initializes RingSocketManager for tests)
|
|
||||||
Server.Network.NetState.Configure();
|
|
||||||
|
|
||||||
// Configure / Initialize
|
|
||||||
TestMapDefinitions.ConfigureTestMapDefinitions();
|
|
||||||
|
|
||||||
// Configure the world
|
|
||||||
World.Configure();
|
|
||||||
|
|
||||||
Timer.Init(0);
|
|
||||||
|
|
||||||
// Configure Races
|
|
||||||
RaceDefinitions.Configure();
|
|
||||||
|
|
||||||
// Load the world
|
|
||||||
World.Load();
|
|
||||||
|
|
||||||
World.ExitSerializationThreads();
|
|
||||||
|
|
||||||
DecayScheduler.Configure();
|
|
||||||
}
|
|
||||||
|
|
||||||
private static int _counter;
|
|
||||||
|
|
||||||
public void Dispose()
|
|
||||||
{
|
|
||||||
_counter++;
|
|
||||||
|
|
||||||
if (_counter > 1)
|
|
||||||
{
|
|
||||||
throw new Exception("NO!");
|
|
||||||
}
|
|
||||||
Timer.Init(0);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue