Release empty CompositeBuffers

[alexwen]

Fix for #237

Release content if not readable.

[NiteshKant]

Thanks @alexwen for digging into this leak, I myself was behind this for a while but not able to point to the cause.

Looking at the code, it appears to me that the check if (content.isReadable()) is unnecessary and we should call invokeContentOnNext() unconditionally. What do you think?

PS: Apologies for the delay in response, I was out on a vacation.

[alexwen]

It would be my preference as well to invokeContentOnNext unconditionally. It may break backwards compatibility in cases where folks might always be expecting the content to be readable, but it is a better behavcior, imo, to have it always emit one ByteBuf.

The current behavior has forced me to put...

response.getContent()
                    .firstOrDefault(UnpooledByteBufAllocator.DEFAULT.buffer(0, 0))

... throughout my code so I can do error handling irrespective of whether or not the server has returned a message.

[NiteshKant]

It may break backwards compatibility in cases where folks might always be expecting the content to be readable, but it is a better behavcior, imo, to have it always emit one ByteBuf.

I agree, this is a better behavior. Do you want to update this PR with this change?

[alexwen]

Attempting this change leads to other issues downstream, it looks like they are originating from ServerSentEvents (as the observable now receives the empty byte buffer before the closing of the stream). I don't have time to look at this too deeply at the moment, but will update the issue if/when I can:

io.reactivex.netty.protocol.http.client.HttpClientTest > testNonChunkingStream FAILED
    java.lang.ClassCastException

io.reactivex.netty.protocol.http.client.HttpClientTest > testChunkedStreaming FAILED
    java.lang.ClassCastException

io.reactivex.netty.protocol.http.client.HttpClientTest > testMultipleChunks FAILED
    java.lang.ClassCastException

io.reactivex.netty.protocol.http.client.HttpClientTest > testMultipleChunksWithTransformation FAILED
    java.lang.ClassCastException
        Caused by: rx.exceptions.OnErrorThrowable$OnNextValue

io.reactivex.netty.protocol.http.client.HttpRedirectTest > testRedirectWithConnPool FAILED
    java.util.concurrent.ExecutionException at HttpRedirectTest.java:161
        Caused by: java.util.NoSuchElementException

[NiteshKant]

@alexwen I have created this PR #242 for the fix to send content onNext unconditionally. It was a little more involved than removing the if-check.

[NiteshKant]

I am closing this PR in favor of #242, thanks for finding this out, anyways!

[alexwen]

No problem. Glad to see there will be a fix soon!