Make character rollbacks impossible - #50
Open
jaenster wants to merge 3 commits into
Open
Conversation
Uncommitted work that was already in the tree, committed separately so the save-rollback changes that follow are readable on their own. process_all_games / dispatch_cleanup are the drive model a real 1.13c server uses: process_all_games is the only caller of ServerGameLoop, which populates newly-activated rooms and reveals them as a client moves. The per-game worker model still gets a character into a world — the join sends the initial view — and then nothing else ever happens. D2Net reports each of the three drain loops once. They differ only by the list index they push, so an ordinal on the wrong one is invisible: packets routed to the loop the engine never asks for accumulate and are never seen, which reads as the engine ignoring the player while the connection stays healthy.
No deployed game server was writing character saves at all. d2gs-native cleared the engine's own -nosave flag and never replaced it; d2host answered fpSaveDatabaseCharacter with a stack-balancing stub. Only the wine-injected 1.14d DLL implemented the slot, and that one filed saves under the wrong key whenever its 16-slot join ring had recycled the player's entry. Every character on the realm read charver=1: created, and never saved since. The store now refuses to go backwards, rather than every path above it being careful. realmd:charver is a compare-and-set token: a load returns the bytes and the version they were at, and a save is accepted only while the store is still at that version. The whole compare-set-increment is one Lua script. A version of zero is accepted whatever the caller expected, because that means redis lost its state and was refilled from postgres, where a live session's bytes are the newest thing in existence. A refused save is never retried; it is obsolete. Character names are now claimed realm-wide instead of per account. Names were unique only within an account, while the seat tables, the retry queue, a departing player's seat release and the Mac engine's own <charname>.d2s all key on the name alone — two accounts with a "Bob" is not a rollback but a character swap. The claim is a table rather than an index on chars, so it cannot fail at startup on data that already contains duplicates. Also closed: deleting a character that is in a game (the server holds it and writes it back, resurrecting it or landing old bytes on a recreated one); releasing a seat by matching a /name suffix over an unordered SMEMBERS, which freed a lock belonging to somebody still playing; a leave now carries the account so the seat is named rather than guessed; and the save-retry queue keyed by name alone, carried no fence, and had no age bound. Saving is continuous everywhere. 1.14d's interval drops from 8192 game frames (~5.5 min) to 512 (~20s) by patching two immediates in UpdateClients. The pre-1.14 engines find the same site by its constant instead of by address — 0x80001fff is the signed-modulo-8192 idiom and nothing else holds that number — and require exactly one match, refusing rather than guessing. All ten shipped D2Game.dll files have exactly one, in both encodings, with the fixup at +7. joinctx moves to packages/gs-seats and is shared by all three servers, which previously had one table, another table, and none. Verified on the real engines under wine: the interval patch applies on all six, a client reaches a world on 1.14d and its charver climbs 1 -> 2 -> 3, and a save replayed at a superseded version is refused with the character left intact. The e2e suite failed about one run in two because pg_isready answers from the temporary server postgres runs during first-boot initdb, which listens on a unix socket while docker has already published the port. It now waits for a query over TCP. 27 consecutive clean runs.
The harness clears its characters from both stores between engines, but a name claim outlives a row deleted behind the realm's back — leaving a name owned by a character that no longer exists. Harmless today, because every run uses the same account per engine and an account may re-claim its own name. It stops being harmless the moment the account naming changes, and the symptom would be a create refused as "name taken" with nothing holding it.
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
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.
Players reported characters being rolled back to an older state. The cause was not a race — no deployed game server was writing character saves at all, and the one server that did wrote them under the wrong key. Every character on the live realm read
charver=1: created, and never saved since.The starting state
d2gs-native-nosaveflag, nothing replaced it;store.putCharhad zero callersd2hostfpSaveDatabaseCharacterwas a stack-balancing stubd2gsrealmdonly ever wrote a save on create/copy/import/upgrade, so a login always restored whatever it had last written itself.The structural fix
Rather than keep making every path above the store careful, the store now refuses to go backwards.
realmd:charverbecomes a compare-and-set token:0is accepted whatever the caller expected — that means redis lost its state and was refilled from postgres, where a live session's bytes are the newest thing in existence;Character names are realm-unique
Names were unique only within an account, while the seat tables, the retry queue, a departing player's seat release, and the Mac engine's own
<charname>.d2sall key on the name alone. Two accounts with a "Bob" is not a rollback but a character swap. Claimed atomically in acharnamestable — a table rather than an index onchars, because an index would be built over data that may already contain duplicates and would fail inside the startup schema bootstrap.Other rollback paths closed
/namesuffix over an unorderedSMEMBERS, which could free a lock belonging to somebody still playing;d2gs-nativeuploaded the previous session's.d2son rejoin (clearFileonly ran in file mode);patch.zigdroppedpoisonedinskip()/rewind(), so a chain that had already refused anexpect()still wrote.Continuous saving
1.14d drops from 8192 game frames (~5.5 min) to 512 (~20s) by patching two immediates in
UpdateClients. The pre-1.14 engines find the same site by its constant rather than by address —0x80001fffis the signed-modulo-8192 idiom and nothing else in a game engine holds that number — and require exactly one match, refusing rather than guessing. All ten shippedD2Game.dllfiles have exactly one, in both encodings, with the fixup always at +7.joinctxmoves topackages/gs-seats, shared by all three servers, which previously had one table, another table, and none.Verification
Run on the real engines under wine, not argued from source:
D2Game.dll autosaves every 512 frames (~20s), was 8192);charverclimbs 1 → 2 → 3, the save landing ~21s after the fetch;-4), version unchanged, character bytes intact.282 unit tests (from 226) and 43/43 e2e, including new
save_fence,realm_unique_names,save_account_keyanddelete_in_gamescenarios.The e2e suite used to fail about one run in two:
pg_isreadyanswers from the temporary server postgres runs during first-bootinitdb, which listens on a unix socket while docker has already published the port. It now waits for a query over TCP — 27 consecutive clean runs.Note on the first commit
6daf188carries uncommitted 1.13c work that was already in the working tree (cooperative drive model, D2Net drain reporting). It is committed separately so this change is readable on its own, butapps/d2host/main.ziginterleaves the two and could not be split cleanly. Drop that commit if it belongs elsewhere.