Coordinate the World and Map session phases from the canonical producer (#787) - #792
Merged
Conversation
Bounded source review at Rust aee29a6 and C++ a5f8da2e for the coordinated map-driven session pass decided on the issue. What C++ does, read at the exact ranges: World.cpp:2704 runs UpdateSessions before :2748 runs MapManager::Update inside the same world tick, with different diffs; WorldSession.cpp:488-540 splits the tails by filter (time sync only in the map pass, query callbacks in both, warden/logout/socket cleanup only in the world pass), so driving the whole Rust driver twice is excluded; LockedQueue.h:82-95 reads the front and does not pop when the check refuses it, so selection is FIFO, head-only and stops at an ineligible head; and MapReference.cpp:22-28 inserts with insertFirst, so the map walk is reverse insertion order rather than sorted GUIDs. What RustyCore has: the ProcessingPlace classification and both filters already exist (wow-handler lib.rs:38, processing.rs:41) and are unconsumed; one session driver per task with its own diff (session_factory.rs:142-215); a canonical map tick holding three synchronous mutexes across the whole update (runtime/map.rs 1596-1634); a MapManager::Update shape that already matches C++ (manager/state_2.rs:426-518) whose MapUpdater runs inline with no async barrier (:604-647); and an existing request/acknowledgement command with a per-request response channel (mailbox/protocol.rs:163-278, shutdown.rs:95-150) whose shape the map-phase request reuses but whose timeout semantics it may not. The contract fixes order, the split point and guard release, serial session execution with justified membership order, exactly-once resumption with the saved diff, filtered FIFO selection with control-channel progress, identity revalidation, failure semantics where an acknowledgement is not completion and a timeout is not permission, the await audit, and the explicit limit that the legacy creature runtime is untouched. It also lists the regressions the macro owes. The plan's stale claim that no ProcessingPlace contract exists is corrected in place with the evidence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The branch carries the tick split, the C++ membership order and the phase-filtered session pass, but the coordinated contract is not satisfied: the acknowledgement deadline does not revoke an admitted operation, the session composition still drives its own clock, the request does not revalidate admission identities or consume the effective diff, and the tick plan is clonable so exactly-once is unenforced. Also corrects the earlier premise that leaving the world makes every Map packet ineligible: Inplace is eligible in both phases regardless of residence, as in C++. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
C++ runs both session passes from the thread that owns the session: World::Update calls UpdateSessions(diff) (World.cpp:2704) and then MapManager::Update(diff), whose maps drive their players' sessions (World.cpp:2748, Map.cpp:669-680). RustyCore sessions own themselves, so the order and the ownership have to travel explicitly. This closes the four correctness gaps the review found in the first attempt. Exactly-once: MapTickPlanLikeCpp is no longer Clone and resume consumes it by value, backed by manager state Idle -> AwaitingSessions(epoch) -> Resuming(epoch) -> Idle. A second begin answers Busy without advancing the shared timer, so a refused diff is still owed to the next accepted tick. Maps carry an incarnation, so one recreated under a reused MapKey during the pass receives neither the phases that follow a dynamic-tree phase it never ran nor the removal the plan recorded for its predecessor. Revocation and quiescence: every request carries a permit whose single atomic transition decides between the session claiming it and the coordinator revoking it. The deadline ends only a pass that never started; a claimed one is waited for and its delay reported, because continuing would overlap live effects. A claimed pass whose end cannot be observed abandons the tick instead of resuming over it. Completion is always an explicit transition, never a destructor, and interruption after start is its own terminal state. Admission: the request freezes coordinator, epoch, phase, permit, PlayerRegistration, PlayerHandle, MapKey, map incarnation and residence revision, and the session revalidates all of them before its first effect, so a replacement incarnation or an away-and-back transfer is refused rather than served. The effective diff feeds the map pass tail (WorldSession.cpp:488-497) even when no packet was dispatched. Composition: phases travel on a per-session rail instead of the command mailbox a pass drains, and are consumed in the task that already owns the WorldSession. The autonomous per-session clock is gone; the producer issues World for every ready session, including those on the character screen, and admits the map tick only once that phase is quiescent. Map->World within one iteration is therefore no longer expressible. Registration is not readiness: the owning task publishes that flag when it parks on the rail and withdraws it when it leaves. Acceptance is not claimed here: the affected suites pass on this tree (wow-map 735, wow-world 3858, world-server 577, all with zero failures), but the remaining contract scenarios, the audit of handlers that await other steps, shutdown exercise and runtime/DB/relogin QA are still open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…udit (#787) The clock trace now says what the producer owns: the world phase of every ready session before the map tick is admitted, and the map-phase requests with their completion boundary. The session clock entry stops claiming a clock of its own: its diff arrives in the request, and its only remaining wake re-reads the shutdown gate. The three new WorldSession members are assigned to reviewed families rather than absorbed by a count. The phase rail joins the delivery family, with the reason it is not the command mailbox recorded there; the coordination flag joins admission/dispatch as an explicit migration boundary. The family totals are recomputed from the regenerated syntax baseline, whose delta is exactly those two fields plus the signatures and bodies this macro changed; the stale expected_unique_field_count of 648 is corrected to the 645 the baseline actually contains. The checkpoint records what the implementation does, the suite numbers measured on this tree, and a finite audit: of 397 registrations, 148 are map-eligible and 54 of those await something persistence-shaped inside the handler. That set is where the permit bounds overlap but not progress, and it is named as open work rather than treated as covered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An admitted pass whose effects cannot be accounted for no longer lets the tick walk on: the phase stops at that participant, and its permit travels to the producer, which holds a barrier across steps until the pass reaches a terminal state. Abandoning a plan was never a clean state, so respawns and DelayedUpdate do not run on that uncertainty. The shutdown handover has one owner. At the gate the session withdraws its readiness, refuses every queued phase before any effect — answering the producer rather than making it wait out a deadline — and drains its own control mailbox, so KickAll and the UpdateSessions(1) flush are observed even without a live producer. A claimed phase is still never cancelled mid-effect; cancellation is honoured at the phase boundary instead, which is a deliberate change from the previous force-cancel behaviour. Provenance is now checked rather than assumed: a per-phase watermark of the producer and step refuses an earlier producer, a retired step and a replay of one already served, none of which a fresh permit or the player's identity can distinguish. The logout decision moves to the end of the world pass, after the packet loop and the query callbacks (WorldSession.cpp:498-503), so a LogoutCancel queued in the same step is seen first. unload_all clears the map incarnations with the maps. The checkpoint's obligation over the 54 map-eligible handlers that await is corrected: they are a candidate inventory, not 54 defects or 54 rewrites. What this macro must fix is the narrower set whose wait needs a session or phase this barrier is holding; an external wait that can progress on its own needs an error/stop policy and evidence, not a callback for symmetry. QuestLogRemoveQuest is classified against C++ (QuestHandler.cpp:439, Player.cpp:15575, Player.cpp:20138, WorkerPool.cpp:533) and left with its existing owner unless this coordination makes it a hard blocker. Suites after the corrections: wow-map 735, wow-world 3865, world-server 579, zero failures each; architecture check, self-test and the syntax-only ratchet pass with the reviewed baseline. Final gate and runtime QA remain open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bounded login/save/relogin gate passes for the candidate with restitution: bot_status 0, login_save_relog_verified true, the original live build restored and serving. It drives a full live session — auth, character enumeration, player login, instance socket, world entry, known spells, stream drain, normal logout on the realm route, offline row with a newer logout_time and the six-family projection retained across two authentications — entirely through the coordinated producer, since the session no longer has a clock of its own. The reputation failure that appeared first was investigated instead of accepted. origin/3.4.3 without this macro performs the same AT_WAR write from the same starting state, so the divergence belongs to the deployed 2026-09-07 build, which predates the branch. The branch matches C++, where LoadFromDB sets AtWar for a hostile rank and keeps needSave when the computed flags differ from the row (ReputationMgr.cpp:766-783). The eight-run sequence that isolates this is recorded, and no database row was edited by hand to reach it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 12, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
C++ runs both session passes from the thread that owns the session:
World::UpdatecallsUpdateSessions(diff)(World.cpp:2704) and thenMapManager::Update(diff), whose maps drive their players' sessions(
World.cpp:2748,Map.cpp:669-680). RustyCore sessions own themselves, sothat order and that ownership have to travel explicitly. This PR makes them
travel, and closes the correctness gaps two review rounds found in the process.
Exactly-once. The tick plan is neither
ClonenorCopyandresumeconsumes it, behind manager state
Idle → AwaitingSessions(epoch) → Resuming(epoch) → Idle. A secondbeginanswersBusywithout advancing theshared timer, so the refused diff is still owed to the next accepted tick. Maps
carry an incarnation, so one recreated under a reused
MapKeyduring the passreceives neither the phases that follow a dynamic-tree phase it never ran nor
the removal recorded for its predecessor — it does still receive
DelayedUpdate, as C++ visits every surviving map (MapManager.cpp:314-317).Revocation, quiescence and the barrier. Each request carries a permit whose
single atomic transition decides between the session claiming it and the
coordinator revoking it at its deadline. A revoked pass provably never ran; a
claimed one is waited for, with the stall reported on every deadline. When a
claimed pass cannot account for its effects, the phase stops at that
participant and the permit travels to the producer, which holds a barrier across
steps: no phase issued and no tick admitted until it resolves, so neither
respawns nor
DelayedUpdaterun on that uncertainty.Admission. The request freezes coordinator, epoch, phase, permit,
PlayerRegistration,PlayerHandle,MapKey, map incarnation, residencerevision and the effective diff, and the session revalidates all of them before
its first effect. A per-phase watermark refuses an earlier producer, a retired
step and a replay of one already served — things a fresh permit and the player's
identity cannot distinguish. A→B→A is caught by the revision.
Composition. Phases travel on a per-session rail instead of the command
mailbox a pass drains, consumed in the task that owns the
WorldSession; theautonomous per-session clock is gone. The producer drives the world phase for
every ready session, character screen included, and admits the map tick only
once that phase is quiescent, so Map→World inside one iteration is no longer
expressible. Registration is not readiness. At shutdown the session takes the
handover: it withdraws readiness, refuses queued phases before any effect and
drains its own control mailbox, so
KickAlland theUpdateSessions(1)flushare observed even without a live producer. A claimed phase is never cancelled
mid-effect; cancellation is honoured at the phase boundary.
Tails. The time sync moves to the map-pass tail with the admitted diff and
the logout decision to the end of the world pass, after the packet loop and the
query callbacks (
WorldSession.cpp:488-503), so aLogoutCancelqueued in thesame step is seen first. Warden has no update call on this path; that is
recorded as absent, not implemented.
Evidence.
validation-v2 final --base origin/3.4.3 --architecture --timingspassed at306269f4on a clean tree: manifest20260912T192723.798686Z-2980204-final.json, 252 s, peak child RSS 2.6 GiB,cargo_jobs=1. Suites:wow-map735,wow-world3865,world-server579,zero failures each; architecture check, self-test and the syntax-only ownership
ratchet pass with the regenerated baseline whose delta was reviewed.
Not claimed, and why this is not yet acceptance. Runtime/DB/relogin QA has
not been run, and composition evidence is still missing for a producer that dies
during shutdown and for finalization after the acknowledgement. Separately, 148
of 397 registrations are map-eligible and 54 of them await something
persistence-shaped: that is a candidate inventory, not 54 defects. What matters
is the narrower set whose wait needs a session or phase this barrier is holding;
QuestLogRemoveQuestis classified against C++ (QuestHandler.cpp:439,Player.cpp:15575,Player.cpp:20138,WorkerPool.cpp:533) and stays with itsexisting owner unless this coordination makes it a hard blocker.
Closes #787
🤖 Generated with Claude Code