AssertionFailure in UnpooledHeapByteBuf. Premature buffer freeing? #794

Closed
coltnz opened this Issue Dec 6, 2012 · 7 comments

3 participants

@coltnz

Hi, I upgraded pom to Alpha8, made no other changes and got test failures. Confirmed against HEAD too.

java.lang.AssertionError
    at io.netty.buffer.UnpooledHeapByteBuf.getLong(UnpooledHeapByteBuf.java:286)
    at io.netty.buffer.SlicedByteBuf.getLong(SlicedByteBuf.java:138)
    at io.netty.buffer.ByteBufUtil.equals(ByteBufUtil.java:128)
    at io.netty.buffer.AbstractByteBuf.equals(AbstractByteBuf.java:884)

286: assert !freed;

When I step through the debugger to see when free() is being called it doesn't seem to happen suggesting a race condition.

The test in question not that it matters much:

   public void testBasicRetrieve() throws Exception {
        HttpResponse r = retrieve(TEST_ID);
        assertEquals(r.getStatus(), HttpResponseStatus.OK);
        assertEquals(r.getContent(), wrappedBuffer(msg));
    }
@normanmaurer
The Netty Project member

@coltnz so you say you see that freed was "not called" before ?

@coltnz

not by me at least :)

@normanmaurer
The Netty Project member

@coltnz do you by any chance access the HttpResponse after the connection was closed ?

@coltnz

Embarrassing but this appears to be user error.

So I assume the semantics are a connection could be closed anytime after the client's messageReceived() returns and Alpha8 prevents any race condition with explicit freed being set and asserted against even if buffer hasn't been reclaimed yet? Yay for defensive programming :)

Client:

    public void messageReceived(ChannelHandlerContext ctx, Object msg) throws Exception {
        if (msg instanceof HttpResponse){
            responseQueue.add((HttpResponse)msg);
        }

Server

private ChannelFuture sendMsg(ChannelHandlerContext ctx, HttpMessage msg, boolean keepAlive) {
        if (keepAlive) {
            msg.setHeader(Names.CONNECTION, Values.KEEP_ALIVE);
        }
        else {
            msg.setHeader(Names.CONNECTION, Values.CLOSE);
        }

      .....

        ChannelFuture writeFuture = ctx.write(msg);
        if (!keepAlive) {
            return writeFuture.addListener(ChannelFutureListener.CLOSE);
        }
        else {
            return writeFuture;
        }

Probably for a discussion for the mailing list but I guess to achieve the API I want for my TestClient I'll have to block or clone? How else can I hand off the response for consumption elsewhere as at the end of a Future?

Future<HttpResponse> send(String resource, String contentType, HttpMethod method, byte[] toSend)
@trustin
The Netty Project member

I believe you should never get an AssertionError in this specific case. It seems like HttpMessageDecoder leaked a slice of its internal buffer. HttpMessageDecoder.java:522?

@trustin trustin was assigned Dec 19, 2012
@trustin trustin closed this in 937c048 Dec 19, 2012
@trustin
The Netty Project member

@coltnz I've just pushed the fix to master. Could you confirm if my fix worked for you?

@coltnz

The bar is green. Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment