Bug Report: IllegalReferenceCountException in tickPlayer causing repeated errors

[usagifiver]

It would be appreciated if you could check and consider this.

Environment

• BetterView Version: 2.1.2 (paper)
• Server: PaperMC 1.21.11
• Occurrence: Error logs are output several times per second for about an hour

Description

The plugin repeatedly throws io.netty.util.IllegalReferenceCountException: refCnt: 0, increment: 1 errors during normal operation. This occurs in the BetterViewManager.tickPlayer() method when processing chunk queue entries.

Error Log

[16:15:42] [Netty Epoll IO #2/ERROR]: [BetterView] Error while ticking player dev.booky.betterview.platform.PaperPlayer@122461be in level dev.booky.betterview.platform.PaperLevel@4733d124
io.netty.util.IllegalReferenceCountException: refCnt: 0, increment: 1
        at io.netty.util.internal.ReferenceCountUpdater.retain0(ReferenceCountUpdater.java:117)
        at io.netty.util.internal.ReferenceCountUpdater.retain(ReferenceCountUpdater.java:104)
        at io.netty.buffer.AbstractReferenceCountedByteBuf.retain(AbstractReferenceCountedByteBuf.java:121)
        at io.netty.buffer.AbstractPooledDerivedByteBuf.init(AbstractPooledDerivedByteBuf.java:64)
        at io.netty.buffer.PooledSlicedByteBuf.newInstance0(PooledSlicedByteBuf.java:52)
        at io.netty.buffer.PooledSlicedByteBuf.newInstance(PooledSlicedByteBuf.java:46)
        at io.netty.buffer.PooledByteBuf.retainedSlice(PooledByteBuf.java:158)
        at io.netty.buffer.PooledByteBuf.retainedSlice(PooledByteBuf.java:153)
        at betterview-paper-2.1.2.jar//dev.booky.betterview.common.BetterViewPlayer.checkQueueEntry(BetterViewPlayer.java:327)
        at java.base/java.util.ArrayDeque.bulkRemove(ArrayDeque.java:926)
        at java.base/java.util.ArrayDeque.removeIf(ArrayDeque.java:900)
        at betterview-paper-2.1.2.jar//dev.booky.betterview.common.BetterViewManager.tickPlayer(BetterViewManager.java:137)
        ...

Probable cause

The error occurs at line 327 in BetterViewPlayer.checkQueueEntry():

ByteBuf finalChunkBuf = chunkBuf.isReadable()
        ? chunkBuf.retainedSlice()  // <- Exception thrown here
        : this.level.getEmptyChunkBuf(chunkPos);

The problem appears to be in the retain() and release() methods of the ChunkQueueEntry record (lines 464-473).

public ChunkQueueEntry retain() {
    this.future.thenApply(ReferenceCountUtil::retain);  // ❌ Problem
    return this;
}

public ChunkQueueEntry release() {
    this.future.thenApply(ReferenceCountUtil::release);  // ❌ Problem
    return this;
}

thenApply() return value is discarded: thenApply returns a new CompletableFuture, but it's ignored, so the retain/release operations are not actually applied to the original buffer.
Doesn't this create a risk of race conditions and early release?

Amendment proposal

Replace thenApply() with whenComplete() to ensure retain/release operations are properly executed:

public ChunkQueueEntry retain() {
    this.future.whenComplete((buf, throwable) -> {
        if (buf != null && throwable == null) {
            ReferenceCountUtil.retain(buf);
        }
    });
    return this;
}

public ChunkQueueEntry release() {
    this.future.whenComplete((buf, throwable) -> {
        if (buf != null && throwable == null) {
            ReferenceCountUtil.release(buf);
        }
    });
    return this;
}

Reproduction

Difficult to reproduce consistently, as it appears to be a race condition that manifests under specific timing conditions with asynchronous chunk loading.

[booky10]

i dont see how your "proposal" would work any different from what we already have
there still is some sort of race condition somewhere, but i'm not sure the "cause" you mention is causing all of this

[usagifiver]

Thank you for your reply.
I'm not sure if the suggested fix will solve the problem entirely, but I believe it will eliminate one possible cause.
My English ability is not good enough to provide a detailed explanation, so please allow me to use an explanation generated by AI.

The Key Problem with thenApply()

The current code:

public ChunkQueueEntry retain() {
    this.future.thenApply(ReferenceCountUtil::retain);
    return this;
}

The issue: thenApply() returns a new CompletableFuture, but we're discarding it. The original this.future remains unchanged.

This means:

// What actually happens:
CompletableFuture<ByteBuf> newFuture = this.future.thenApply(ReferenceCountUtil::retain);
// newFuture contains the retained ByteBuf
// BUT: we discard newFuture and keep using this.future (which has the non-retained ByteBuf)
return this; // Returns ChunkQueueEntry with the ORIGINAL future

Later, when checkQueueEntry() calls entry.future.getNow(), it gets the original, non-retained ByteBuf, which may have already been released by Netty.

Why whenComplete() Also Doesn't Fully Solve It

My initial proposal with whenComplete():

this.future.whenComplete((buf, throwable) -> {
    if (buf != null && throwable == null) {
        ReferenceCountUtil.retain(buf);
    }
});

While this does call retain() as a side effect, this.future.getNow() still returns the original ByteBuf reference, not a newly retained one. The refCount is incremented, but we're not guaranteed to be using a properly retained reference in the calling code, and race conditions can still occur depending on timing.

The Correct Fix

Since ChunkQueueEntry is a record (immutable), retain() must return a new ChunkQueueEntry with a transformed future:

public ChunkQueueEntry retain() {
    return new ChunkQueueEntry(
        this.chunkPos,
        this.future.thenApply(buf -> {
            if (buf != null) {
                return ReferenceCountUtil.retain(buf);
            }
            return null;
        })
    );
}

This ensures:

1. A new ChunkQueueEntry is created with a new Future that contains the retained ByteBuf
2. When entry.future.getNow() is called, it gets the retained ByteBuf
3. The ByteBuf lifecycle is properly managed across thread boundaries

The Race Condition Timeline

Current (broken):

Server Thread:   queueEntry.retain() → Returns same entry (future unchanged)
Chunk Thread:    ByteBuf created (refCount=1) → Future completes
Netty Thread:    Sends packet → Releases ByteBuf (refCount=0)
Server Thread:   entry.future.getNow() → Gets released ByteBuf
                 ❌ retainedSlice() → IllegalReferenceCountException

With fix:

Server Thread:   queueEntry.retain() → Returns NEW entry with transformed future
Chunk Thread:    ByteBuf created (refCount=1) → Original future completes
                 → Transformed future applies retain() (refCount=2)
Netty Thread:    Sends packet → Releases ByteBuf (refCount=1) ✅
Server Thread:   entry.future.getNow() → Gets retained ByteBuf (refCount still > 0)
                 ✅ retainedSlice() succeeds

Is This the Complete Cause?

This is the primary structural issue that allows the race condition to occur. The current implementation makes it impossible to properly retain ByteBufs before they're accessed, which is why you're seeing refCnt: 0 errors.

However, I recommend auditing other areas where ByteBuf lifecycle is managed to ensure there are no other edge cases. But fixing this should resolve the reported error.

For your reference, we have also attached the results of the AI ​​analysis.
ISSUE_14_TECHNICAL_EXPLANATION.md

[booky10]

Please stop posting ai slop as a possible explanation for this - the AI is wrong, the bytebuf itself will get mutated

I will look into this later today myself

[usagifiver]

I apologize for the confusion caused by the AI ​​analysis. As you pointed out, we did not properly understand the ByteBuf mutation.

I appreciate you taking the time to look into this issue yourself. As I don't have a deep understanding of the plugin's architecture and normal behavior patterns, I should not have posted speculative explanations.

I'll close this issue and let you investigate it properly. Thank you for your work on this plugin, and I apologize for any inconvenience caused.

[booky10]

this is still a valid issue, you don't need to close it