Skip to content

Fix two server crashes: Team Objective stack overflow under --desync-patch, access violation in the combat hooks - #64

Draft
wiseLlama0 wants to merge 5 commits into
Chiv2-Community:implement-rust-hooks-maximalfrom
wiseLlama0:fix-server-crashes
Draft

wiseLlama0 wants to merge 5 commits into
Chiv2-Community:implement-rust-hooks-maximalfrom
wiseLlama0:fix-server-crashes

Conversation

@wiseLlama0

Copy link
Copy Markdown

Two crash fixes found while running community servers on v1.0.0-RC2. Based on implement-rust-hooks-maximal at the RC2 tag

1. Stack overflow on Team Objective maps with --desync-patch

Symptom. With --desync-patch on, loading a Team Objective map ends in EXCEPTION_STACK_OVERFLOW about five seconds later. Repro on an empty server: RCON servertravel To_DarkForest. With the patch off, the same travel works and the match reaches InProgress. This is a known issue with TO maps in RC1

Cause. The ProcessEvent hook in resolvers/desync.rs already drops ClientSetCameraMode ("event spam (deadlock)"), but it is compiled out with { || false }, so the block never runs.

Fix.

  • The hook is active whenever --desync-patch is given. Every event now passes the detour, so the ClientSetCameraMode check uses the FName comparison index; the string conversion happens once per distinct function and is cached per thread. The per-event Class::Function debug log is dropped for the same reason.
  • A per-thread depth guard in the same hook: beyond 300 nested calls (UE's own script limit is 120), the event is dropped and logged as Class::Function at error level instead of overflowing the stack. It never fired once ClientSetCameraMode was blocked; it's a safety net for the next loop of this kind.

Result. With the block active, the TO map loaded, the plugin logged four ClientSetCameraMode blocked lines, the match reached InProgress, and two players then played a TO match on it. Travel back to an FFA map worked.

There may be more crashes that come up in Team Objective maps, but in order to fix those we need to hit them by actually playing the Team Objective maps, which this allows.

2. Access violation in the damage and kill hooks

Symptom. ACCESS VIOLATION ... READ addr=0x100000440 at the first instruction of TArray<u16>::copy_to_string, reached from the combat resolvers in resolvers/gameplay.rs. One server logged 26+ crash dumps with it between 2026-09-22 and 09-28, and a second server crashed with it too. The same fault crashed a client running the released RC2 DLL after a kill credited to an actor without a player state (a mod's damage volume).

Cause. The resolvers name the instigator, causer and victim of every damage and kill event by following pointers to an APlayerState and copying PlayerNamePrivate:

  • The bad value is 0x0000000100000100, from TBLCharacter.LastPlayerState, which the plugin declares as a raw pointer. Read as two i32s it is 0x100 and 1, which looks like a TWeakObjectPtr (index, serial number) rather than an address. It is aligned and in the user-mode range, so nothing rejected it.
  • copy_to_string copied the text through ReadProcessMemory but read the length directly. With PlayerNamePrivate at 0x338 and the length at +8, that read faults at exactly 0x100000440. A dangling pointer to a destroyed object could fail the same way.
  • Related: object_inherits_from matched class names by substring, so "Character" also accepted classes such as CharacterDeathcam_C, which were then read through the ATBLCharacter layout. LastPlayerState was read from any "Character", although it exists only on ATBLCharacter.

Fix.

  • First commit: exact class-name matching, a bounded superclass walk, and a cheap pointer pre-check. This alone does not stop the crash, because the bad value passes the pre-check.
  • Second commit, which stops it:
    • ue::try_read<T>, a copy through ReadProcessMemory that fails instead of faulting.
    • FString::copy_to_string reads its header that way and rejects a negative length, a length above capacity, or one above 2^20 code units.
    • FUObjectArray::is_live: the object's InternalIndex, read safely, must name a GUObjectArray slot that holds this pointer and is neither pending kill nor unreachable (the check IsValid makes).
    • The combat resolvers require is_live for every actor, controller, player state and damage-source pointer before reading it, read LastPlayerState only on a real TBLCharacter, and log a warning naming the field when they reject one.

Result. On a server under load (29 minutes, 7 joins, 1,868 damage events, 321 kills) the plugin logged ignoring PlayerState 0x100000100 from TBLCharacter.LastPlayerState: not a live object twice and kept running. That is the value that crashed it before.

Testing

  • cargo build --release and cargo test --release pass. Each commit compiles on its own (cargo check) with the same 9 warnings as RC2.
  • These changes have run on our servers since 2026-09-22 (TO fix, class-name matching) and 2026-09-30 (liveness checks), inside fork builds that also carry unrelated changes (diagnostic flags, server-mod spawning, chat logging). In this branch, gameplay.rs and src/ue/ are identical to what ran apart from comments. The ProcessEvent hook takes the same path under --desync-patch, without our diagnostic flags. This exact build has not been run on a server yet, which is why the PR is a draft.

Not included

  • LastPlayerState as a weak pointer: declaring it as TWeakObjectPtr and resolving it through GUObjectArray would give those events a name instead of a warning. Not done yet.
  • Diagnostic flags we used to find the TO loop (--log-events, --block-events). Happy to send them separately if they're wanted.

🤖 Generated with Claude Code

wiseLlama0 and others added 4 commits October 4, 2026 08:30
Problem
-------
The damage and kill resolvers in resolvers/gameplay.rs pick the memory
layout to read an object with from object_inherits_from, which walks
the object's class chain and compared each class name by substring.
"Character" therefore also matched unrelated classes such as
CharacterDeathcam_C, and those objects were then read as an
ATBLCharacter: values at ATBLCharacter field offsets were treated as
pointers and followed, although the object has a different layout.
Pointers were checked only for null before being dereferenced, and the
class-chain walk had no bound.

Fix
---
- object_inherits_from compares class names exactly.
- looks_like_pointer, a cheap pre-filter (non-null, 8-byte aligned,
  inside the user-mode address range), is applied to the object, to
  each super_struct, and to the player-state and controller pointers
  before they are dereferenced.
- The class-chain walk stops after 32 levels.

Why this fixes it
-----------------
With exact matching, only classes that really derive from the named
class take that code path, so the ATBLCharacter layout is only applied
to objects that have it. The pre-filter rejects null and obviously
invalid values before any read, and the bound keeps a corrupted class
chain from looping.

The pre-filter does not prove that an object lives at an address: a
value such as 0x100000000 passes it. The next commit adds that check.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Problem
-------
Servers crashed with "ACCESS VIOLATION ... READ addr=0x100000440" at
the first instruction of TArray<u16>::copy_to_string, called from the
damage and kill resolvers while they read a player's name
(APlayerState::PlayerNamePrivate).

The value behind it is 0x0000000100000100, read from
ATBLCharacter::LastPlayerState, which the plugin declares as a raw
pointer. Read as two 32-bit values it is 0x100 and 1, which looks like
a TWeakObjectPtr (object index, serial number) rather than an address.
It is aligned and in the user-mode range, so looks_like_pointer accepts
it. PlayerNamePrivate is at offset 0x338, and copy_to_string read the
string's length (offset 8) directly, so it faulted at
0x100000100 + 0x338 + 8 = 0x100000440. Any dangling pointer to a
destroyed object could fail the same way.

The resolvers also read LastPlayerState from any object matching
"Character", although the field exists only on ATBLCharacter and lies
past the end of a plain ACharacter.

Fix
---
- ue::try_read<T> copies a value through ReadProcessMemory and returns
  None instead of faulting.
- FString::copy_to_string reads its header (data pointer, length,
  capacity) with try_read, and rejects a negative length, a length above
  capacity, or one above 2^20 code units before allocating.
- FUObjectArray::is_live reports whether a pointer is a live UObject:
  the object's InternalIndex, read with try_read, must name a
  GUObjectArray slot that holds this exact pointer and is neither
  pending kill nor unreachable. This is the test UE's IsValid makes.
- The resolvers require is_live for every actor, controller, player
  state and damage-source pointer before reading through it.
- LastPlayerState is read only when the object is a TBLCharacter.
- A rejected player state is logged as a warning naming the field it
  came from, e.g. "TBLCharacter.LastPlayerState".

Why this fixes it
-----------------
A pointer is followed only if GUObjectArray holds that exact object at
the index the object claims, so weak-pointer bit patterns, garbage and
pointers to destroyed objects are all rejected before any field is
read. If a bad pointer still reaches copy_to_string, try_read makes the
header read fail with an error instead of an access violation.

With this change, a server under load logged "ignoring PlayerState
0x100000100 from TBLCharacter.LastPlayerState: not a live object" twice
and kept running; before, that value crashed it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Problem
-------
With --desync-patch on, loading a Team Objective map crashes the server
with EXCEPTION_STACK_OVERFLOW: about five seconds after the map loads
on an empty server, within a minute with players on. It reproduces on
an empty server with "servertravel To_DarkForest"; with the patch off,
the same travel works. The crash stack repeats a cycle of six frames
that passes through UObject::ProcessEvent.

The plugin already contains a fix for this kind of loop: a ProcessEvent
hook in resolvers/desync.rs that drops ClientSetCameraMode, commented
"event spam (deadlock)". But the hook's enable condition is
{ || false }, so it is never installed and the event is never blocked.

Fix
---
- The ProcessEvent hook is installed whenever --desync-patch is given.
- The hook now sees every event in the game, so the
  "is this ClientSetCameraMode?" check uses the function's FName
  comparison index. The name is converted to a string once per distinct
  function and the result is cached per thread, instead of allocating a
  string for every event.
- The per-event "Class::Function" debug log is removed for the same
  reason.

Why this fixes it
-----------------
The desync patch makes UNetDriver::GetNetMode report a dedicated
server while the process runs as a listen server with a local player
controller. In that state ClientSetCameraMode feeds an event loop, which
the existing block was written to stop; it only needed to run.

With the hook installed, the Team Objective map loads, the plugin logs
"ClientSetCameraMode blocked" four times, the match reaches
InProgress, and two players then played a match on it. Travel back to
a free-for-all map also works.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Problem
-------
The Team Objective crash fixed by the previous commit was a stack
overflow from an event loop running through UObject::ProcessEvent.
Blocking ClientSetCameraMode ends that loop, but the net-mode mismatch
the desync patch creates could start other loops of the same kind. Each
would take the server down with nothing in the log naming the event
involved.

Fix
---
The ProcessEvent hook counts its nesting depth per thread. At 300
nested calls it drops the event instead of calling the original and
logs "recursion depth N reached; dropping Class::Function" at error
level. The counter is decremented by a guard object, so it stays
correct on every return path.

Why this fixes it
-----------------
Every iteration of such a loop passes through ProcessEvent, so the
counter rises with the loop. Legitimate nesting stays far below 300
(UE's own script recursion limit is 120), so only a runaway loop
reaches it. Dropping the event there unwinds the recursion before the
stack runs out, and the log line names the event that needs a
permanent fix.

The guard has not tripped since ClientSetCameraMode is blocked; it is a
safety net.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Knutschbert referenced this pull request Oct 10, 2026
…acking and reporting for ProcessEvent calls (largely based on PR #62 by @wiseLlama0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants