ChannelBufferInputStream throws IndexOutOfBoundsException

[mvysny]

Hi guys, thank you for an excellent library!
During some very specific conditions, I can always trigger the following error. Can you please
take a quick look at it, perhaps you can spot the problem very easily. If not, I will try to provide sample data to trigger the issue, but it will take some time as it is not easy
to simulate these very specific conditions.

java.lang.IndexOutOfBoundsException: Invalid index: 174351, maximum: 14
at org.jboss.netty.buffer.CompositeChannelBuffer.componentId(CompositeChannelBuffer.java:684)
at org.jboss.netty.buffer.CompositeChannelBuffer.getBytes(CompositeChannelBuffer.java:231)
at org.jboss.netty.buffer.AbstractChannelBuffer.readBytes(AbstractChannelBuffer.java:339)
at org.jboss.netty.handler.codec.replay.ReplayingDecoderBuffer.readBytes(ReplayingDecoderBuffer.java:318)
at org.jboss.netty.buffer.ChannelBufferInputStream.readFully(ChannelBufferInputStream.java:166)
at org.jboss.netty.buffer.ChannelBufferInputStream.readFully(ChannelBufferInputStream.java:161)
at com.innovatrics.jobcluster.common.messages.DataUtils.readBytes(DataUtils.java:46)
at com.innovatrics.jobcluster.common.messages.MessageFrame.(MessageFrame.java:35)
at com.innovatrics.jobcluster.server.connector.tcp.MessagingNettyServer$MessageFrameDecoder.decode(MessagingNettyServer.java:62)
at com.innovatrics.jobcluster.server.connector.tcp.MessagingNettyServer$MessageFrameDecoder.decode(MessagingNettyServer.java:58)
at org.jboss.netty.handler.codec.replay.ReplayingDecoder.callDecode(ReplayingDecoder.java:502)
at org.jboss.netty.handler.codec.replay.ReplayingDecoder.messageReceived(ReplayingDecoder.java:487)
at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:268)
at org.jboss.netty.channel.Channels.fireMessageReceived(Channels.java:255)
at org.jboss.netty.channel.socket.nio.NioWorker.read(NioWorker.java:91)
at org.jboss.netty.channel.socket.nio.AbstractNioWorker.processSelectedKeys(AbstractNioWorker.java:385)
at org.jboss.netty.channel.socket.nio.AbstractNioWorker.run(AbstractNioWorker.java:256)
at org.jboss.netty.channel.socket.nio.NioWorker.run(NioWorker.java:35)
at java.util.concurrent.ThreadPoolExecutor$Worker.runTask(ThreadPoolExecutor.java:886)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:908)
at java.lang.Thread.run(Thread.java:662)

[mvysny]

This bug is present in Netty 3.5.2 and 3.5.3, maybe also in other versions. Perhaps this bug is related to Issue #445?

[normanmaurer]

@mvysny would it be possible to share a unit test to reproduce ?

[mvysny]

I'm very sorry but I can't reproduce it on a regular basis. Perhaps this issue is reproducible only if TCP packets arrive with a certain size?

[mvysny]

The problem seems to be caused by some kind of inconsistency between the value returned by CompositeChannelBuffer.readableBytes() and internal CompositeChannelBuffer state (the indices[] array). I tried to run the server in debug mode, to check for these inconsistencies, but so far it works flawlessly in debug mode.

[normanmaurer]

@mvysny let me know if you find out more..

[yogeshvedpathak]

Hi, I ma also seeing this issue pretty consistently. Do we have any further information on this issue? Let me know if you want me to collect any specific data.

-Yogesh

[normanmaurer]

@yogeshvedpathak some way to reproduce it would be very helpful.

[normanmaurer]

@mvysny @yogeshvedpathak I suspect there is a bug in the componentId(..) method of CompositeChannelBuffer..

https://github.com/netty/netty/blob/3/src/main/java/org/jboss/netty/buffer/CompositeChannelBuffer.java#L665

[yogeshvedpathak]

@normanmaurer I thought that CompositeChannelBuffer is a culprit. But then the code is very similar to 3.2.7. The difference is most exceptions have message 3.5.3. Then I started thinking that it may be FrameDecoder. This class has substantial changes to cumulating buffer. Is it possible that copy buffer in that code is not setting read index properly? I am trying to setup a test environment where I can reproduce it and set a breakpoint.

[yogeshvedpathak]

Looks like i the issue is indeed in CompositeChannelBuffer. I made following change to CompositeChannelBuffer .componentId()

663c663
< if (index < indices[lastComponentId + 1]) {

        if (index <= indices[lastComponentId + 1]) {

I am not able to reproduce the issue with this change. However I like you to take a look at make sure that this change do not add new issue. Because I am not entirely familiar with this code. Please also let me know if change is good so I can start using it in production. We can't wait until next netty release.

[yogeshvedpathak]

Also note that, I see the issue when i write around 100MB data on the connection.

[normanmaurer]

@yogeshvedpathak let me see if I can write a unit test to reproduce and then verify the fix

[normanmaurer]

@yogeshvedpathak I was not able to reproduce it with a unit test yet. Can you explain why you thing this change is needed ?

[yogeshvedpathak]

Here is the snapshot for CompositeChannelBuffer:

index = 294565 (componentId's parameter)

lastComponentId = 240
indices size = 242
indices[241] = 294565
readerIndex = 294565
writerIndex = 294565
markedReaderIndex = 294553
markedWriterIndex = 0
components.size = 241

If i make that if condition <= then then lastComponentId is returned instead of throwing exception. I hope this helps.

[trustin]

Looking from the snapshot, the capacity of the buffer is 294565 and you are trying to access the 294565th element. Isn't it natural that you get IOOBE? What is the capacity of the buffer?

[normanmaurer]

@yogeshvedpathak did you see @trustin 's comment ?

[yogeshvedpathak]

@normanmaurer yes I did. And it make sense. Actually i did run into IOOBE during another test because of that change. I don't have new update. I will spend some more time on this on Monday.

[normanmaurer]

@yogeshvedpathak ok let me know once you have more details

[yogeshvedpathak]

So i was not able to reproduce it in the unit test. However the problem is very consistant on actual server. Also our server disconnects the client if there is no IO for few seconds. So debugging this issue with the breakpoint set is hard. I added some logging to the CompositeChannelBuffer. I see that this happens right after setComponenet is called with indices size 730 and components size 729. and call to componentId fails when accessing index 1052764. So logically we are trying to access the index which i not set? In general I don't understand if it is okay to have no synchronization in ChannelBuffer code. Can you suggest anything to investigate this further?

[yogeshvedpathak]

@normanmaurer after adding some more loggin I was able to figure out exact condition when this exception is thrown. This happens if CompositeChannelBuffer.readBytes(ByteBuffer) is called with zero buffer being passed is zero length. I am not sure what is/should be contract of this method. For not i fixed my code to not call readBytes with empty buffer. Let me know what you think.

[normanmaurer]

@yogeshvedpathak I tried to reproduce it was a test case but was not able todo.. Could you submit one ?

[voidmain]

@yogeshvedpathak is correct. However, it is more than just a zero length byte array being passed to readBytes(). The issue requires that the CompositeChannelBuffer readerIndex be sitting at the exact end of the entire buffer and at that point an attempt is made to read an empty byte array.

The fix here is simple. At the top of all the getBytes methods an if-statement causes the method to exit. Here's an example for getBytes(int index, byte[] dst, int dstIndex, int length) on line 230:

public void getBytes(int index, byte[] dst, int dstIndex, int length) {
    if (length == 0) {
        return;
    }

    int componentId = componentId(index);
    ...

The unit test will look something like this (pseudo code):

byte[] b = new byte[0]
CompositeChannelBuffer buf = new CompositeChannelBuffer(Big, [ChannelBuffer(10), ChannelBuffer(10)], true)
buf.getBytes(20, b, 0, 0)

[normanmaurer]

Thanks,

Will take care today ;)

Sent from my iPhone. Excuse any typos....

Am 22.09.2012 um 16:04 schrieb voidmain notifications@github.com:

@yogeshvedpathak is correct. However, it is more than just a zero length byte array being passed to readBytes(). The issue requires that the CompositeChannelBuffer readerIndex be sitting at the exact end of the entire buffer and at that point an attempt is made to read an empty byte array.

The fix here is simple. At the top of all the getBytes methods an if-statement causes the method to exit. Here's an example for getBytes(int index, byte[] dst, int dstIndex, int length) on line 230:

public void getBytes(int index, byte[] dst, int dstIndex, int length) {
if (length == 0) {
return;
}

int componentId = componentId(index);
...

The unit test will look something like this (pseudo code):

byte[] b = new byte[0]
CompositeChannelBuffer buf = new CompositeChannelBuffer(Big, [ChannelBuffer(10), ChannelBuffer(10)], true)
buf.getBytes(20, b, 0, 0)
—
Reply to this email directly or view it on GitHub.

[normanmaurer]

@voidmain @yogeshvedpathak thanks again for all the details.. I just pushed a fix which will be part of 3.5.8.Final. Please let me know if you still have any issues...