Merge branch 'claude/quirky-payne-46a2e6' into claude/latest-commits-cb0c8f
This commit is contained in:
Erik
commit
629d83411d
5 changed files with 274 additions and 32 deletions
|
|
@ -209,7 +209,8 @@ reached — no `seal-refused` spam, no crash from the original bug) and the
|
|||
session exits gracefully every time (`[session] graceful logout confirmed`,
|
||||
zero leases at disposal). Full `airborne-transition True` confirmation is
|
||||
blocked by a SEPARATE, newly-discovered, pre-existing defect — see #368 below
|
||||
— not by anything in this issue's scope. A diagnostic-only run with #368's
|
||||
— not by anything in this issue's scope. (2026-08-10 update: #368 is CLOSED
|
||||
at `b7f59923`; the airborne residual survived that fix and is now #370.) A diagnostic-only run with #368's
|
||||
guard temporarily neutralized (never shipped, reverted before commit)
|
||||
confirmed the #365 fix produces the correct behavior once past that unrelated
|
||||
blocker: full hydration, the jump-probe policy running to completion, exit
|
||||
|
|
@ -241,9 +242,33 @@ Environment). `HeadlessDiagnosticWriter.Failure` now emits `errorDetail`
|
|||
|
||||
## #368 — Headless scheduler's async tick loop can run collision-generation calls on different threads, tripping `EnsureCollisionMutationThread`
|
||||
|
||||
**Status:** OPEN — filed 2026-08-10 during #365's end-to-end verification.
|
||||
Explicitly OUT OF SCOPE for #365 — orthogonal mechanism, not mentioned
|
||||
anywhere in that diagnosis.
|
||||
**Status:** CLOSED 2026-08-10 — fixed in `b7f59923`. One dedicated update
|
||||
thread (`acdream-headless-update`, spawned by `HeadlessProcessHost.RunAsync`)
|
||||
now owns the whole session-side lifecycle: `Start()` (the live connect
|
||||
transaction, where the first collision-mutating call can already happen),
|
||||
every scheduler turn, and the post-loop resource captures.
|
||||
`HeadlessProcessScheduler.Run(CancellationToken)` replaced `RunAsync` — the
|
||||
same deadline math and counters, but fully synchronous on the calling
|
||||
thread, with waits going through one rearmed `TimeProvider` timer
|
||||
signalling an event instead of `await Task.Delay`, so the loop never leaves
|
||||
its thread. `EnsureCollisionMutationThread` is untouched — the invariant it
|
||||
guards is real, and the headless host now satisfies it the same way the
|
||||
graphical host's game-loop thread does. Zero shared Runtime changes, so the
|
||||
graphical host is unaffected by construction. Evidence: new
|
||||
`ProcessHostRunsStartAndEveryTickOnOneDedicatedUpdateThread` test (RED
|
||||
pre-fix — Start ran on the caller's thread, ticks migrated to pool
|
||||
workers), Headless suite 97/97, full Release suite 12,554 / 4 skips / 0
|
||||
failures, and three live jump-probe runs against local ACE that each
|
||||
crossed `[wake] begin gen=1` — the exact point all three pre-fix runs
|
||||
quarantined — with zero faults, 204–205 hydrated entities, policy
|
||||
completion, ACE-confirmed graceful logout, converged disposed samples, and
|
||||
exit 0. The jump-airborne timeout this issue carried as an open question
|
||||
persists 3/3 on the fixed tree — the threading-artifact hypothesis is
|
||||
refuted; split off as #370.
|
||||
|
||||
Filed 2026-08-10 during #365's end-to-end verification. Explicitly OUT OF
|
||||
SCOPE for #365 — orthogonal mechanism, not mentioned anywhere in that
|
||||
diagnosis.
|
||||
|
||||
**Symptom:** a real headless run against live ACE (`jump-probe` policy,
|
||||
`ACDREAM_PROBE_PARK=1`) that survives long enough for the local player's own
|
||||
|
|
@ -292,21 +317,62 @@ directly, never through the real `HeadlessProcessScheduler.RunAsync` await
|
|||
loop — so none of them exercise this path either. A genuine coverage gap,
|
||||
not a regression from a specific commit.
|
||||
|
||||
**NOT fixed here.** A proper fix needs its own investigation (e.g. a
|
||||
dedicated single update thread for the scheduler's tick loop, or redesigning
|
||||
the thread-affinity invariant for verified multi-thread-safe callers) and
|
||||
must be verified against the GRAPHICAL host too — out of reach for the
|
||||
session that found this (constrained not to launch the graphical client).
|
||||
Do not "fix" this by loosening or removing `EnsureCollisionMutationThread`'s
|
||||
check — it guards a real invariant elsewhere in the physics/collision
|
||||
system, and a workaround shape here is exactly what CLAUDE.md's
|
||||
no-workarounds rule forbids without explicit approval.
|
||||
**Why the dedicated-thread shape (not an invariant redesign):** the guard
|
||||
is only the ENFORCER — Runtime's single-update-thread contract is
|
||||
documented all over (`RuntimeEntityDirectory` "single update-thread
|
||||
authority", `RuntimeLocalPlayerPhysicsPublicationState` "this single
|
||||
Runtime update thread", `RuntimePlacementProjectionChannel`), mostly
|
||||
without enforcement. An async tick loop violates the whole contract, not
|
||||
one check; weakening the check would have silenced the one place that
|
||||
noticed while leaving every unenforced assumption exposed, and would have
|
||||
degraded the guard for the graphical host where migration is always a bug.
|
||||
The dedicated thread fixes the entire class. Start() had to move onto the
|
||||
same thread too: the first collision-mutating call can land during
|
||||
connect, and a caller-thread Start would bind the guard there and trip the
|
||||
very first dedicated tick. Disposal legitimately stays on the lifecycle
|
||||
thread — `ResetSessionPhysics`'s doc comment designs for exactly that, and
|
||||
every prior graceful-teardown run (including the quarantined ones)
|
||||
exercised it.
|
||||
|
||||
**Repro:** run the `jump-probe` policy against local ACE with
|
||||
**Repro (historical):** run the `jump-probe` policy against local ACE with
|
||||
`ACDREAM_PROBE_PARK=1` for long enough that the local player's own landblock
|
||||
collision generation spans more than a couple of scheduler ticks (the
|
||||
default case against a real DAT-loaded landblock).
|
||||
|
||||
## #370 — Headless jump-probe: the released jump never registers as airborne (proven NOT a threading artifact)
|
||||
|
||||
**Status:** OPEN — filed 2026-08-10 during #368's fix verification.
|
||||
|
||||
**Symptom:** with #368 fixed (one dedicated update thread, thread
|
||||
migration provably gone — the new affinity test pins it), the `jump-probe`
|
||||
policy reaches `[jump-probe] releasing jump (fire)` and then reports
|
||||
`TIMEOUT waiting for airborne after jump fire -- the released jump never
|
||||
registered as airborne.` Reproduced 3/3 on the fixed tree at
|
||||
`lb=0x1134FFFF`; the #365 session saw the identical timeout in its
|
||||
guard-neutralized diagnostic run at `lb=0x0904FFFF`
|
||||
(`docs/research/2026-08-10-365-headless-hydration-diagnosis.md` §8), so it
|
||||
is location-independent and pre-existing. The run still exits 0 with
|
||||
ACE-confirmed graceful logout — the probe treats its timeout as
|
||||
completion, so nothing quarantines.
|
||||
|
||||
**What this refutes:** the #365 OUTCOME's hypothesis that the timeout was
|
||||
"plausibly a downstream artifact of the same unsynchronized-thread
|
||||
condition" — threads are now single and the timeout persists unchanged.
|
||||
This is a distinct defect (or probe-expectation gap) in the headless jump
|
||||
path: either the policy's charge/release never becomes a real jump on the
|
||||
wire/local motion, or the airborne signal the policy polls is never set on
|
||||
the headless projection. Everything before the fire provably works
|
||||
(hydration 204–205 entities, `local player present`, movement owner
|
||||
publication per #365's fix).
|
||||
|
||||
**Where to start:** `JumpProbeHeadlessBotPolicy` (what it reads as
|
||||
"airborne"), the J5.4 `RuntimeLocalPlayerMovementState` jump intent/outbound
|
||||
cadence path, and whether the graphical host's airborne transition has a
|
||||
presentation-side dependency the headless projection lacks.
|
||||
|
||||
**Repro:** #368's recipe (jump-probe vs local ACE, `ACDREAM_PROBE_PARK=1`);
|
||||
the timeout fires seconds after `releasing jump (fire)`.
|
||||
|
||||
## #369 — Unconfirmed whether retail's floating chat windows share the main window's currently-selected talk-focus channel
|
||||
|
||||
**Status:** OPEN — filed 2026-08-10, Campaign CH slice CH6b (register row
|
||||
|
|
|
|||
|
|
@ -447,3 +447,16 @@ Every real (unmodified) run's session tore down gracefully
|
|||
(`[session] graceful logout confirmed`, zero entities/leases at the final
|
||||
`disposed` sample) regardless of which way it exited — `testaccount` was
|
||||
never left in a stuck state by this work.
|
||||
|
||||
## 9. ADDENDUM (2026-08-10, #368 fix session)
|
||||
|
||||
#368 is CLOSED at `b7f59923`: one dedicated headless update thread now owns
|
||||
Start, every scheduler turn, and the post-loop captures; the scheduler loop
|
||||
is synchronous with TimeProvider-timer event waits (no `Task.Delay`
|
||||
resumption migration; zero shared Runtime changes). Three live jump-probe
|
||||
runs on the fixed tree each crossed `[wake] begin gen=1` cleanly with
|
||||
204–205 hydrated entities and graceful exits. The §8 open question is now
|
||||
answered: the `airborne-transition True` timeout PERSISTS 3/3 with threading
|
||||
provably single, so the "downstream artifact of the unsynchronized-thread
|
||||
condition" hypothesis is refuted — the residual is a distinct pre-existing
|
||||
defect, filed as #370.
|
||||
|
|
|
|||
|
|
@ -156,10 +156,44 @@ internal sealed class HeadlessProcessHost : IDisposable
|
|||
Credential = source.Credential,
|
||||
};
|
||||
|
||||
internal async Task<HeadlessExitCode> RunAsync(
|
||||
internal Task<HeadlessExitCode> RunAsync(
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
ObjectDisposedException.ThrowIf(_disposed, this);
|
||||
// #368: Runtime's gameplay owners require ONE update thread for a
|
||||
// session's whole lifetime — collision generations bind to the
|
||||
// first mutating thread and refuse migration. The graphical host
|
||||
// satisfies that with its game-loop thread; this dedicated thread
|
||||
// is the headless equivalent. Start (the live connect
|
||||
// transaction), every scheduler turn, and the post-loop captures
|
||||
// all execute here. Only disposal stays on the lifecycle thread,
|
||||
// which the Runtime teardown path explicitly supports (see
|
||||
// ResetSessionPhysics's own doc comment).
|
||||
var completion = new TaskCompletionSource<HeadlessExitCode>(
|
||||
TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
var thread = new Thread(() =>
|
||||
{
|
||||
try
|
||||
{
|
||||
completion.SetResult(
|
||||
RunOnUpdateThread(cancellationToken));
|
||||
}
|
||||
catch (Exception error)
|
||||
{
|
||||
completion.SetException(error);
|
||||
}
|
||||
})
|
||||
{
|
||||
IsBackground = true,
|
||||
Name = "acdream-headless-update",
|
||||
};
|
||||
thread.Start();
|
||||
return completion.Task;
|
||||
}
|
||||
|
||||
private HeadlessExitCode RunOnUpdateThread(
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
foreach (HeadlessSessionHost session in _sessions)
|
||||
{
|
||||
RuntimeSessionStartResult started = session.Start();
|
||||
|
|
@ -189,8 +223,7 @@ internal sealed class HeadlessProcessHost : IDisposable
|
|||
|
||||
try
|
||||
{
|
||||
await _scheduler.RunAsync(cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
_scheduler.Run(cancellationToken);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
when (cancellationToken.IsCancellationRequested)
|
||||
|
|
|
|||
|
|
@ -147,8 +147,31 @@ internal sealed class HeadlessProcessScheduler
|
|||
_observationPeriodTicks);
|
||||
}
|
||||
|
||||
internal async Task RunAsync(CancellationToken cancellationToken)
|
||||
/// <summary>
|
||||
/// Drives every session's deadlines on the CALLING thread until
|
||||
/// cancellation or policy completion, returning normally in both
|
||||
/// cases. The caller must dedicate one thread for a process's whole
|
||||
/// run: Runtime's gameplay owners (collision generations foremost,
|
||||
/// via <c>RuntimePhysicsState.EnsureCollisionMutationThread</c>) bind
|
||||
/// to the first mutating thread and refuse migration, and an awaited
|
||||
/// timer loop in a SynchronizationContext-free host resumes on
|
||||
/// arbitrary ThreadPool workers — which tripped that guard whenever a
|
||||
/// collision generation spanned two waits (#368). The waits below go
|
||||
/// through one rearmed <see cref="TimeProvider"/> timer signalling an
|
||||
/// event, so the loop never leaves its thread. A stale timer callback
|
||||
/// from an abandoned wait can set the event early; that only costs
|
||||
/// one extra pass over the deadline math, which re-sleeps.
|
||||
/// </summary>
|
||||
internal void Run(CancellationToken cancellationToken)
|
||||
{
|
||||
using var wake = new ManualResetEventSlim(false);
|
||||
using ITimer timer = _timeProvider.CreateTimer(
|
||||
static state => ((ManualResetEventSlim)state!).Set(),
|
||||
wake,
|
||||
Timeout.InfiniteTimeSpan,
|
||||
Timeout.InfiniteTimeSpan);
|
||||
WaitHandle[] waitHandles =
|
||||
[wake.WaitHandle, cancellationToken.WaitHandle];
|
||||
while (!cancellationToken.IsCancellationRequested
|
||||
&& HasActiveSession())
|
||||
{
|
||||
|
|
@ -169,11 +192,9 @@ internal sealed class HeadlessProcessScheduler
|
|||
: _timeProvider.GetElapsedTime(now, deadline);
|
||||
delay = NormalizeTimerDelay(delay);
|
||||
Interlocked.Increment(ref _waitCount);
|
||||
await Task.Delay(
|
||||
delay,
|
||||
_timeProvider,
|
||||
cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
wake.Reset();
|
||||
timer.Change(delay, Timeout.InfiniteTimeSpan);
|
||||
WaitHandle.WaitAny(waitHandles);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
using System.Collections.Concurrent;
|
||||
using System.Diagnostics;
|
||||
using System.Net;
|
||||
using AcDream.Core.Net;
|
||||
using AcDream.Core.Net.Messages;
|
||||
|
|
@ -198,7 +200,7 @@ public sealed class HeadlessProcessSchedulerTests
|
|||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SystemTimerCadenceDoesNotBusyLoopBetweenTurns()
|
||||
public void SystemTimerCadenceDoesNotBusyLoopBetweenTurns()
|
||||
{
|
||||
var operations = new FixtureSessionOperations();
|
||||
using HeadlessSessionHost session =
|
||||
|
|
@ -214,14 +216,10 @@ public sealed class HeadlessProcessSchedulerTests
|
|||
using var cancellation =
|
||||
new CancellationTokenSource(TimeSpan.FromMilliseconds(250));
|
||||
|
||||
try
|
||||
{
|
||||
await scheduler.RunAsync(cancellation.Token);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
when (cancellation.IsCancellationRequested)
|
||||
{
|
||||
}
|
||||
// Run executes on the calling thread and returns normally when
|
||||
// the token cancels (#368 moved thread ownership to the process
|
||||
// host; the scheduler seam itself is synchronous).
|
||||
scheduler.Run(cancellation.Token);
|
||||
|
||||
HeadlessSchedulerSnapshot snapshot =
|
||||
scheduler.CaptureSnapshot();
|
||||
|
|
@ -440,6 +438,63 @@ public sealed class HeadlessProcessSchedulerTests
|
|||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task ProcessHostRunsStartAndEveryTickOnOneDedicatedUpdateThread()
|
||||
{
|
||||
// #368: Runtime's gameplay owners (RuntimePhysicsState's collision
|
||||
// generations foremost) require one update thread for a session's
|
||||
// whole lifetime — the graphical host provides its game-loop thread;
|
||||
// the headless host must provide an equivalent. Start() (the live
|
||||
// connect transaction) and every subsequent tick must share one
|
||||
// thread that is NOT the RunAsync caller's, across real timer waits.
|
||||
var configuration = new HeadlessConfiguration
|
||||
{
|
||||
Version = 1,
|
||||
Sessions =
|
||||
[
|
||||
Descriptor(
|
||||
"update-thread",
|
||||
"update-thread-stdin"),
|
||||
],
|
||||
};
|
||||
var operations = new ThreadRecordingSessionOperations();
|
||||
using var diagnostics = new StringWriter();
|
||||
using var host = new HeadlessProcessHost(
|
||||
configuration,
|
||||
HeadlessPathSet.Resolve(new HeadlessPathOverrides()),
|
||||
new System.IO.StringReader(
|
||||
"update-thread-password" + Environment.NewLine),
|
||||
diagnostics,
|
||||
operations);
|
||||
using var cancellation = new CancellationTokenSource();
|
||||
|
||||
int callerThread = Environment.CurrentManagedThreadId;
|
||||
Task<HeadlessExitCode> run = host.RunAsync(cancellation.Token);
|
||||
// Occupy the calling thread until ticks have crossed several timer
|
||||
// waits — pre-fix, resumptions migrate to other pool threads while
|
||||
// this thread is provably unavailable to them. The await happens
|
||||
// only after the measurement window closes.
|
||||
var stopwatch = Stopwatch.StartNew();
|
||||
while (operations.TickCount < 5
|
||||
&& stopwatch.Elapsed < TimeSpan.FromSeconds(10))
|
||||
{
|
||||
Thread.Sleep(1);
|
||||
}
|
||||
cancellation.Cancel();
|
||||
HeadlessExitCode result = await run;
|
||||
|
||||
Assert.Equal(HeadlessExitCode.Success, result);
|
||||
Assert.True(
|
||||
operations.TickCount >= 5,
|
||||
$"Expected at least 5 ticks, observed {operations.TickCount}.");
|
||||
int updateThread = operations.ConnectThreadId;
|
||||
Assert.NotEqual(0, updateThread);
|
||||
Assert.NotEqual(callerThread, updateThread);
|
||||
Assert.All(
|
||||
operations.TickThreadIds,
|
||||
id => Assert.Equal(updateThread, id));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ThrowingPolicyQuarantinesOnlyItsOwnSession()
|
||||
{
|
||||
|
|
@ -609,6 +664,60 @@ public sealed class HeadlessProcessSchedulerTests
|
|||
}
|
||||
}
|
||||
|
||||
private sealed class ThreadRecordingSessionOperations
|
||||
: ILiveSessionOperations
|
||||
{
|
||||
private int _connectThreadId;
|
||||
private readonly ConcurrentQueue<int> _tickThreadIds = new();
|
||||
|
||||
internal int ConnectThreadId =>
|
||||
Volatile.Read(ref _connectThreadId);
|
||||
internal int TickCount => _tickThreadIds.Count;
|
||||
internal IReadOnlyCollection<int> TickThreadIds => _tickThreadIds;
|
||||
|
||||
public IPEndPoint ResolveEndpoint(string host, int port) =>
|
||||
new(IPAddress.Loopback, port);
|
||||
|
||||
public WorldSession CreateSession(IPEndPoint endpoint) =>
|
||||
new(endpoint);
|
||||
|
||||
public void Connect(
|
||||
WorldSession session,
|
||||
string user,
|
||||
string password) =>
|
||||
Volatile.Write(
|
||||
ref _connectThreadId,
|
||||
Environment.CurrentManagedThreadId);
|
||||
|
||||
public CharacterList.Parsed GetCharacters(
|
||||
WorldSession session) =>
|
||||
new(
|
||||
0u,
|
||||
[
|
||||
new CharacterList.Character(
|
||||
0x50000001u,
|
||||
"Headless",
|
||||
0u),
|
||||
],
|
||||
[],
|
||||
11,
|
||||
"account",
|
||||
true,
|
||||
true);
|
||||
|
||||
public void EnterWorld(
|
||||
WorldSession session,
|
||||
int activeCharacterIndex)
|
||||
{
|
||||
}
|
||||
|
||||
public void Tick(WorldSession session) =>
|
||||
_tickThreadIds.Enqueue(Environment.CurrentManagedThreadId);
|
||||
|
||||
public void DisposeSession(WorldSession session) =>
|
||||
session.Dispose();
|
||||
}
|
||||
|
||||
private abstract class FixturePolicy : IHeadlessBotPolicy
|
||||
{
|
||||
public virtual bool IsComplete => false;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue