fix(net): pin the array behind PacketReader(byte[]) — dangling pointer on every compressed gump

[JD-Ultima]

The bug

PacketReader(byte[] buff, bool dyn) in Razor/Network/Packet.cs takes its pointer inside a fixed block and keeps using it after the block ends, while holding no reference to the array:

public PacketReader(byte[] buff, bool dyn)
{
    fixed (byte* p = buff)
        m_Data = p;          // escapes the fixed block
    m_Length = buff.Length;
    ...
}

fixed pins only for the statement it wraps. Its only caller is Packet.GetCompressedReader(), where the inflated buffer is a local — so from the moment the reader is returned, nothing keeps the array alive or in place and the GC is free to move or collect it.

Handlers.CompressedGump (the 0xDD handler) then reads through that pointer while allocating heavily in between — Regex.Split over the layout, an int.Parse per match, and a string per string-table entry. A collection landing inside that window is not a remote possibility.

The result is either silently wrong gump data, or an access violation in JIT code. An AV is a corrupted-state exception, so the handler's catch {} never sees it and the host process dies. Under ClassicUO that surfaces as the client exiting with 0xC0000005 and no managed stack trace — intermittent, and more frequent on shards that redraw gumps often.

Reproduction

A harness that loads a released Razor.exe by reflection, builds a PacketReader the way GetCompressedReader does, forces a compacting GC, then reads the buffer back:

Build	Trials returning wrong bytes
1.9.77.0 (released binary)	200 / 200
with this patch	0 / 200

I first found this from a minidump of a ClassicUO crash: AV reading a misaligned address, ExceptionAddress in no loaded module (JIT heap), mscorlib.ni.dll + clr.dll on the faulting stack, and the managed heap holding a constructed "Attempted to read or write protected memory" exception. The client log ended mid-0xDD.

The fix

Pin the array with a GCHandle for the reader's lifetime and free it in a finalizer.

The byte* constructor is the per-packet hot path and its buffer is owned and already pinned by the caller (ClassicUO.OnRecv/OnSend and OSI build their readers inside their own fixed blocks and never let them escape), so it allocates no handle and calls GC.SuppressFinalize to stay off the finalizer queue. Nothing changes for that path.

PacketReader(byte[]) is the only place in the codebase where such a pointer escapes — I checked every fixed block in Razor/.

Builds clean against master (net472, x64).

---

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

The finalizer design adds per-packet overhead and retains pinned buffers nondeterministically.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

• Avoid finalization overhead and prolonged buffer pinning · New

What changed in this PR

Fixes dangling pointers in array-backed PacketReader instances used for compressed packet data.

Changes:

• Pins array-backed buffers with GCHandle.
• Adds finalizer-based handle cleanup.
• Suppresses finalization for caller-owned pointer buffers.

File	Description
Razor/​Network/​Packet.cs	Adds managed-buffer pinning and lifetime cleanup.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.