Skip to content

fix(EntityEnhancements): guard eye detours against re-entrancy (StackOverflow crash) - #115

Open
Prefix wants to merge 1 commit into
Kxnrl:masterfrom
Prefix:fix/pointviewcontrol-eye-reentrancy
Open

fix(EntityEnhancements): guard eye detours against re-entrancy (StackOverflow crash)#115
Prefix wants to merge 1 commit into
Kxnrl:masterfrom
Prefix:fix/pointviewcontrol-eye-reentrancy

Conversation

@Prefix

@Prefix Prefix commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

The four PointViewControl eye detours resolve managed wrappers while running inside the hooked native function:

  • entry: _entityManager.MakeEntityFromPointer<IPlayerPawn>(pEntity)
  • ShouldOverrideEye()pawn.GetCameraService()?.ViewEntityBaseEntity.Create()

If any of that work reaches the hooked function again, the detour re-enters itself and recurses until the stack is exhausted. The process then dies with System.StackOverflowException, which .NET does not permit catching — the server goes down with no recoverable error and no log line.

Evidence

Two managed crash reports from a production server, six days apart, identical signature:

System.StackOverflowException
  Sharp.Modules.EntityEnhancements.Modules.PointViewControl.LinuxGetEyePosition(IntPtr)
    Sharp.Core.Managers.EntityManager.MakeEntityFromPointer<__Canon>(IntPtr)
      Sharp.Core.GameEntities.BaseEntity.Create(IntPtr)
        "repeated": "0x4340"        <- 17,216 frames

The second report shows 17,208 frames of the same function. Both are on a build that already contains the vtable-resolved hook from #109, so that change does not address this.

Note ShouldOverrideEye's second operand is evaluated on every call whenever _viewControlEntities is empty — i.e. on every server that has no point_viewcontrol entity but does call GetEyePosition.

Why it is rarely seen

The hook installs unconditionally at Init(), but the detour body only runs when something actually calls GetEyePosition/GetEyeAngles. Servers whose plugins never call them never execute it. It reproduces reliably on a server whose plugins call IBasePlayerPawn.GetEyePosition() frequently.

Fix

A [ThreadStatic] re-entrancy flag — if the detour is already active on this thread, go straight to the trampoline.

  • thread-static because the hooked function is not main-thread-only
  • try/finally so the flag is cleared even if wrapper resolution throws; MakeEntityFromPointer throws InvalidOperationException("nullptr") when the pointer does not resolve, which would otherwise latch the flag permanently
  • applied to all four detours (Linux + Windows, position + angles) since they share the same shape

Behaviour is unchanged for every non-re-entrant call. Builds clean against master (0 Warning(s), 0 Error(s)).

Possible follow-up (not in this PR)

The hook is installed even when the map contains no point_viewcontrol entity, so on most servers it is pure overhead on a hot path. Deferring installation until the first such entity spawns would avoid the cost entirely — happy to do that separately if you would like it.

…ainst re-entrancy

The four PointViewControl eye detours resolve managed wrappers while running
inside the hooked native function -- MakeEntityFromPointer<IPlayerPawn>() on
entry, and ShouldOverrideEye()'s CameraService.ViewEntity, which calls
BaseEntity.Create(). When any of that work reaches the hooked function again
the detour re-enters itself and recurses until the stack is exhausted.

The process then dies with System.StackOverflowException, which .NET does not
allow to be caught -- the server goes down with no recoverable error.

Observed in production on a server whose plugins call IBasePlayerPawn
.GetEyePosition() frequently: two crash reports, ~17,200 identical frames of
LinuxGetEyePosition each, innermost frames MakeEntityFromPointer ->
BaseEntity.Create. Both on a build that already contains the vtable-resolved
hook from Kxnrl#109, so that change does not address this.

Servers that never call GetEyePosition never execute the detour body and so
never see it, which is why this has gone unnoticed.

Fix is a [ThreadStatic] re-entrancy flag: if the detour is already active on
this thread, go straight to the trampoline. Thread-static because the hooked
function is not main-thread-only. try/finally so the flag is cleared even if
wrapper resolution throws -- MakeEntityFromPointer throws
InvalidOperationException("nullptr") when the pointer does not resolve.

Applied to all four detours (Linux + Windows, position + angles) since they
share the same shape.
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.

1 participant