Pooled direct ByteBuf copy from `ClientboundCustomPayloadPacket.getInternalData()` is never released on the modded-payload path (per-packet direct-memory leak; client-side OOM on high-traffic packs)

Minecraft Version: 1.20.1

Forge Version: 47.4.18 (reproduced there; the relevant net.minecraftforge.network code is unchanged through the current 47.4.22, so it is present there too). Netty 4.1.82 as bundled.

Logs: attached: leak_records_excerpt.txt — 3 complete netty LEAK records from a 61-minute session that produced 7,282 of them, all sharing one Created-at stack. The full leak-detector session log, Eclipse MAT reports, and heap dumps are available on request.

Steps to Reproduce:

1. Client with one or more SimpleChannel-using mods installed, connected to a dedicated server running the same mods (any mods that sync regularly will do; more/chattier mods only make it faster).
2. Add -Dio.netty.leakDetection.level=advanced to the client JVM args, or watch the java.nio:type=BufferPool,name=direct MBean (e.g. via JConsole).
3. Play or simply stand idle. The direct pool grows monotonically in proportion to received custom-payload traffic and never returns; with leak detection on, LEAK: records appear with the Created-at stack shown below.
4. To confirm it is pinned rather than uncollected: run jcmd <pid> GC.run — the pool does not shrink. Once -XX:MaxDirectMemorySize is exhausted the client is kicked with OutOfMemoryError: Cannot reserve N bytes of direct buffer memory.

Plain Forge alone cannot exhibit the leak — no modded payloads take the affected path — which we suspect is why past reports of this symptom were hard to pin down. If a from-scratch reproducer is preferred, see the offer at the end: a ~20-line mod that registers one SimpleChannel, sends packets client-bound in a loop, and logs the direct-pool MBean.

Description of issue:

On the client, every incoming custom-payload packet handled through Forge's SimpleChannel leaks one pooled direct ByteBuf. Forge makes a defensive copy of each custom payload (ClientboundCustomPayloadPacket.getInternalData() → new FriendlyByteBuf(f_132030_.copy())), stores it on NetworkEvent.payload, dispatches it to registered handlers, and then returns without ever calling release() on that copy. Pooled direct memory is reclaimed only by release(), never by GC, so the pool grows monotonically for the life of the session. On a heavily-modded pack (several hundred mods, high SimpleChannel sync-packet volume) we measured roughly 70 MB/min of direct-memory growth per client. Vanilla is unaffected (no modded payloads take this path); lightly-modded setups leak too slowly to notice, but the omission is present for any SimpleChannel traffic.

We have a one-injection client-side fix and full leak-detector + heap-dump evidence, and we would be glad to open a PR. We understand 1.20.1 is in maintenance and may only take critical fixes — this is filed primarily as data plus a fix offer, not a request for priority.

Why it reproduces on essentially every SimpleChannel mod

Nothing about the leak is mod-specific. Any mod that registers a SimpleChannel and receives packets triggers it, because the un-released buffer is Forge's own copy, allocated before the mod's handler is ever reached. The leak rate is simply proportional to SimpleChannel packet volume: constant-rate sync traffic (capability sync, movement sync, etc.) makes idle and active sessions leak at nearly the same rate. In a leak-detector session we captured 7,282 LEAK: ByteBuf.release() was not called before it's garbage-collected records in a single 61-minute session (~2/second), and every one of them shared one identical "Created at:" allocation stack — the copy site below. The "recently accessed" frames varied by whichever mod's message happened to be in flight (Forge handshake, a capability-sync packet, a player-variables-sync packet, etc.), confirming the mods are consumers of the leaked buffer, not the cause.

Code walk (file:line from the 1.20.1 branch / our runtime decompile)

1. The owned copy is minted. ClientboundCustomPayloadPacket.getInternalData() returns new FriendlyByteBuf(f_132030_.copy()). With netty 4.1.82, PooledDirectByteBuf.copy() → alloc().directBuffer() — a pooled direct buffer at refCnt == 1. This memory returns to the arena only via release().

2. NetworkEvent takes ownership but no one drops it. NetworkEvent.<init> (NetworkEvent.java, ~lines 35–40) stores payload.getInternalData() as this.payload. A grep of the entire net.minecraftforge.network package finds no release() for this copy anywhere on the dispatch path — not in NetworkEvent, NetworkInstance.dispatch, SimpleChannel.networkEventListener, or IndexedMessageCodec.consume/tryDecode. The only release() on the receive path is on the original wire buffer (the shouldRelease path), which works correctly.

3. Vanilla's release is bypassed for modded payloads. Forge's patched ClientPacketListener early-returns for modded custom payloads (patch line ~75) before vanilla's try { ... } finally { copy.release() } for the payload copy. So vanilla's own release, which would have covered this, never runs for the modded branch. (In our woven-runtime decompile the early return sits at the modded-payload branch, ahead of vanilla's finally release block — consistent with the source.)

Net effect: the copy is created, handed to SimpleChannel → IndexedMessageCodec.consume, decoded into a POJO that handlers receive via enqueueWork, and then the buffer is simply dropped with refCnt == 1. Pooled, so GC never reclaims it.

Origin. The copying was introduced deliberately in PR #9157 (the LAN-corruption / MC-121884 fix). The missing release on the modded-payload branch appears to be an unacknowledged side effect of that change.

Leak-detector

leak_records_excerpt.txt

-Dio.netty.leakDetection.level=advanced produced the 7,282 records above, all sharing this Created-at stack (abbreviated):

LEAK: ByteBuf.release() was not called before it's garbage-collected.
Created at:
  ...ClientboundCustomPayloadPacket.getInternalData()      // FriendlyByteBuf(copy())
  net.minecraftforge.network.NetworkEvent$ServerCustomPayloadEvent.<init>
  net.minecraftforge.network.NetworkDirection.getEvent
  net.minecraftforge.network.NetworkInstance.dispatch
  net.minecraftforge.network.NetworkHooks.onCustomPayload
  ...

Corroborating measurements, all reproducible:

• Heap dump at a ~1.5 GB pinned pool (Eclipse MAT): 1,006 of 1,006 leaked 4 MiB direct buffers are io.netty.buffer.PoolChunk backing memory owned by one PooledByteBufAllocator on the client↔server NioSocketChannel — i.e. netty's connection arena, not any mod's own pool.
• Forced-GC discrimination: jcmd GC.run at a 1.484 GB pool freed only ~82 MB → the memory is reachable/pinned, not dead-but-uncollected, ruling out a GC-starvation explanation.
• Runtime bytecode export (-Dmixin.debug.export=true): the woven ClientPacketListener / SimpleChannel.networkEventListener bytecode matches stock on the leak path — no third-party mixin adds or removes a release there.

Symptom-wise, this mechanism would also explain community reports of mod-count-scaling OOMs where no single mod is ever identifiable (e.g. #10010, closed undiagnosed) — though our verified evidence is client-side only.

Serverbound asymmetry (please note before fixing)

Our fix (release event.getPayload() after SimpleChannel.networkEventListener) is verified safe on the client only. When we loaded the identical jar on a dedicated server, every joiner was kicked ~2 s after connect with:

io.netty.util.IllegalReferenceCountException: refCnt: 0, decrement: 1

That is a double-free: on the serverbound receive path the payload evidently is not a freshly-owned copy the way the clientbound path is (consistent with serverbound/login-query packets returning their internal buffer directly rather than a copy()). So the two directions have different buffer-ownership semantics, and a release that is correct clientbound is a double-release serverbound. We report this only as observed behavior; we did not chase down the exact serverbound ownership in source. The proper upstream fix should release exactly where the owned copy is minted (direction-aware), ideally with a try/finally around the dispatch so it also covers the handler-throws path.

Our fix and measured results

ascendra_netleak_fix v0.1 — a ~40-line Mixin mod, single injection:

@Mixin(value = SimpleChannel.class, remap = false)
public abstract class SimpleChannelMixin {
    @Inject(method = "networkEventListener", at = @At("RETURN"), remap = false)
    private void releasePayload(NetworkEvent event, CallbackInfo ci) {
        ByteBuf p = event.getPayload();
        if (p != null && p.refCnt() > 0) p.release();
    }
}

Guards: null check (the ChannelRegistrationChangeEvent branch has no payload) and refCnt() > 0 (defensive against a consumer or netty having already released). networkEventListener has exactly one RETURN opcode, so the release fires once per completed dispatch. The buffer is fully consumed synchronously before RETURN (handlers get decoded POJOs via enqueueWork, never the buffer), so post-consume release is safe.

Measured on the same client that leaked ~70 MB/min:

• Slope: ~70 MB/min → ~0.1 MB/min over a 21-minute session (>99% reduction); the pool now breathes (drops between bursts) instead of ratcheting up.
• Join/leave test: pre-fix each cycle permanently added ~250–300 MB (floors marching 20 → 900 → 1170 → 1484 MB); with the fix each logout snaps the floor back down, residual ~10 MB/cycle (likely benign arena high-water growth; not separately proven).
• ~3.5-hour multi-client session, zero memory kicks (previously OOM'd in ~1–2 h).

Known v0.1 gap: because it's @At("RETURN") rather than a true try/finally, if a registered handler throws, RETURN isn't reached and that one packet leaks. All of our measured ~70 MB/min was the normal-return path, which the injection covers completely, but a proper fix should use finally semantics.

Offer

For reproduction without a large pack: we can supply a minimal reproducer mod (~20 lines) that registers one SimpleChannel, sends packets client-bound in a loop, and logs the java.nio:type=BufferPool,name=direct MBean — the pool climbs by exactly the payload volume and never returns.

We can also attach the full leak-record dump, MAT path-to-GC-roots reports, and the heap dump. Thanks for maintaining this for as long as you have.