diff --git a/docs/ISSUES.md b/docs/ISSUES.md index 1f09c0ab..0408c390 100644 --- a/docs/ISSUES.md +++ b/docs/ISSUES.md @@ -97,6 +97,45 @@ Copy this block when adding a new issue: --- +## #255 — Two RetailDatLoader concurrency tests measured the thread pool, not the loader + +**Status:** DONE — 2026-07-28; the pair now runs on dedicated threads +**Severity:** LOW (test infrastructure only; no production defect) +**Filed:** 2026-07-28 +**Component:** tests / xUnit parallelism, content loaders + +**Description:** `AcDream.Content.Tests.Vfx.RetailDatLoaderTests.AnimationCache_CoalescesSameDidAndAllowsUnrelatedReadsInParallel` +and `.PhysicsScriptLoader_AllowsConcurrentFirstReads` failed on **both** legs of +`portable-headless` in run 30392764012. They were previously invisible: the +ubuntu leg died in Core.Net (#254) before reaching `AcDream.Content.Tests`, and +the windows leg died at an even earlier step (the misplaced `sudo apt-get`), and +the workflow's test loop exits on the first failing project. + +**Root cause:** both assert on `RawDatabase.MaxConcurrentReads` after issuing two +`Task.Run` reads, each of which blocks 40 ms in `Thread.Sleep`. A pair of pool +work items does not guarantee two workers are ever in flight: on a low-core or +saturated pool the second queues behind the first, the two reads run back to +back, `MaxConcurrentReads` stays 1, and the assertion fails for a reason that has +nothing to do with the loader. Same family as #252 and #254 — a test asserting a +real-time/parallel property under an unbounded-parallelism test host. + +**Evidence:** clean on Windows locally (6/6 full-suite runs, 124/124 each). +Reproduced by pinning the suite to two CPUs on Ubuntu 24.04 (`taskset -c 0,1`): +**5 failures in 6 runs**, the same two tests every time. + +**Fix:** a private `InParallel` helper starts both callbacks with +`TaskCreationOptions.LongRunning` on `TaskScheduler.Default`, which asks for a +thread each, so the concurrency the assertions measure is actually offered. **No +assertion changed** — they still fail if the loader serialises. The two +coalescing tests were moved onto the same helper deliberately: two callers +genuinely in flight is the situation coalescing exists for, where a sequential +pair only ever exercised a cache hit. 10/10 clean under the same two-CPU pin +afterwards. + +**Files:** `tests/AcDream.Content.Tests/Vfx/RetailDatLoaderTests.cs`. + +--- + ## #254 — Logout confirmation wait overran its timeout on a starved thread pool **Status:** DONE — 2026-07-28; monotonic deadline added to the synchronous drain diff --git a/docs/plans/2026-07-27-vulkan-campaign.md b/docs/plans/2026-07-27-vulkan-campaign.md index 426e1585..21179789 100644 --- a/docs/plans/2026-07-27-vulkan-campaign.md +++ b/docs/plans/2026-07-27-vulkan-campaign.md @@ -3320,6 +3320,22 @@ structural `spirv-dis` compare. The pinned NuGet native is already the pin. the token, which still bounds the asynchronous wait. Ten of ten clean under the same two-CPU pin afterwards. The test was not touched. +**And one the fixes uncovered.** With (3) and (4) gone, `portable-headless` +reached `AcDream.Content.Tests` for the first time on either operating system — +the workflow's test loop exits on the first failing project, so the windows leg +had never got past the apt step and the ubuntu leg had never got past Core.Net — +and two `RetailDatLoaderTests` cases failed on both. Same family again, and +again not the campaign's: they assert `MaxConcurrentReads` after issuing two +`Task.Run` reads that each block 40 ms, and a pair of pool work items is not two +workers in flight. Reproduced at 5 failures in 6 under the two-CPU pin, clean 6/6 +on Windows. Both pairs now start with `TaskCreationOptions.LongRunning` so the +concurrency the assertions measure is actually offered; **no assertion changed**, +and the two coalescing cases get stronger for it, since a sequential pair only +ever exercised a cache hit. 10/10 clean under the pin. Filed as #255. + +**Net effect on the V9 row:** the job is green, and so is the whole workflow — +its first fully green run is the evidence the row was waiting for. + ### 5.4 The null-target `BeginPass` divergence (V4c) — ✅ DISCHARGED at V6k > **Closed 2026-07-28 by V6k commit 2 (`eb7e6b4e`); see §5.5.16.** The answer is diff --git a/tests/AcDream.Content.Tests/Vfx/RetailDatLoaderTests.cs b/tests/AcDream.Content.Tests/Vfx/RetailDatLoaderTests.cs index 76426298..1c994277 100644 --- a/tests/AcDream.Content.Tests/Vfx/RetailDatLoaderTests.cs +++ b/tests/AcDream.Content.Tests/Vfx/RetailDatLoaderTests.cs @@ -15,6 +15,41 @@ namespace AcDream.Content.Tests.Vfx; public sealed class RetailDatLoaderTests { + /// + /// Runs both callers on dedicated threads rather than thread-pool work + /// items. + /// + /// + /// These tests measure whether the LOADER serialises reads, and the + /// interesting ones assert on MaxConcurrentReads. A pair of + /// work items does not guarantee two + /// workers are ever in flight at once: on a saturated or low-core pool - + /// four cores on a hosted CI runner - the second item simply queues behind + /// the first, both 40 ms reads run back to back, and the assertion fails + /// for a reason that has nothing to do with the loader. Observed exactly + /// that way in CI on both operating systems, and reproduced locally by + /// pinning the suite to two CPUs (5 failures in 6). + /// on the default + /// scheduler asks for a thread per callback, which makes the concurrency + /// the assertions measure actually available. The assertions themselves are + /// unchanged and still fail if the loader serialises - and the coalescing + /// cases get stronger, because two callers genuinely in flight is the + /// situation coalescing exists for, where a sequential pair only ever + /// exercised a cache hit. + /// + private static Task InParallel(Func first, Func second) => + Task.WhenAll( + Task.Factory.StartNew( + first, + CancellationToken.None, + TaskCreationOptions.LongRunning, + TaskScheduler.Default), + Task.Factory.StartNew( + second, + CancellationToken.None, + TaskCreationOptions.LongRunning, + TaskScheduler.Default)); + private sealed class RawDatabase : IDatDatabase { private readonly Dictionary _entries = new(); @@ -263,15 +298,15 @@ public sealed class RetailDatLoaderTests portal.Add(secondDid, AnimationBytes(secondDid)); var loader = new RetailAnimationLoader(portal); - DatAnimation?[] firstPair = await Task.WhenAll( - Task.Run(() => loader.LoadAnimation(firstDid)), - Task.Run(() => loader.LoadAnimation(firstDid))); + DatAnimation?[] firstPair = await InParallel( + () => loader.LoadAnimation(firstDid), + () => loader.LoadAnimation(firstDid)); Assert.Same(firstPair[0], firstPair[1]); Assert.Equal(1, portal.TotalReads); - await Task.WhenAll( - Task.Run(() => loader.LoadAnimation(secondDid)), - Task.Run(() => loader.LoadAnimation(0x03010026u))); + await InParallel( + () => loader.LoadAnimation(secondDid), + () => loader.LoadAnimation(0x03010026u)); Assert.True(portal.MaxConcurrentReads >= 2); Assert.Equal(3, portal.TotalReads); @@ -291,9 +326,9 @@ public sealed class RetailDatLoaderTests portal.Add(secondDid, second); var loader = new RetailPhysicsScriptLoader(portal); - await Task.WhenAll( - Task.Run(() => loader.LoadPhysicsScript(firstDid)), - Task.Run(() => loader.LoadPhysicsScript(secondDid))); + await InParallel( + () => loader.LoadPhysicsScript(firstDid), + () => loader.LoadPhysicsScript(secondDid)); Assert.Equal(2, portal.MaxConcurrentReads); Assert.Equal(2, portal.TotalReads); @@ -319,9 +354,9 @@ public sealed class RetailDatLoaderTests portal.Add(scriptDid, bytes); var loader = new RetailPhysicsScriptLoader(portal); - DatPhysicsScript?[] loaded = await Task.WhenAll( - Task.Run(() => loader.LoadPhysicsScript(scriptDid)), - Task.Run(() => loader.LoadPhysicsScript(scriptDid))); + DatPhysicsScript?[] loaded = await InParallel( + () => loader.LoadPhysicsScript(scriptDid), + () => loader.LoadPhysicsScript(scriptDid)); Assert.All(loaded, Assert.NotNull); Assert.Same(loaded[0], loaded[1]);