DemuxingProtocolCodecFactory.doDecode is called even after the buffer is empty.

• Key: DIRMINA-45
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Trustin Lee
• Reporter: Alex
• Affects Version/s: 0.7.1
• Fix Version/s: 0.7.2

Description

I am not sure if it is a bug or I just misunderstood something.

I am implementing a protocol and use Demuxing* classes.
When I return MessageDecoder.OK from my MessageDecoder's decode method, MINA tries repeatedly calls this method again. If I return MessageDecoder.NEED_DATA everything goes fine.
So, I would expect completely reversal behaviour.

I checked CumulativeProtocolDecoder and DemuxingProtocolCodecFactory classes.
In CumulativeProtocolDecoder.decode it says that doDecode is invoked repeatedly until it returns false. Fine. But what "false" means here? I would guess that it means that buffer was completely decoded and no more data is needed. But DemuxingProtocolCodedFactory.doDecode returns true if decoder returned MessageDecoder.OK and false if decoder returned MessageDecoder.NEED_DATA. Isn't wrong here?

Alex

Comments

[Trustin Lee]

This behavior is correct.

Let's assume that there are 3 and half messages in the buffer.

'OK' means 'I decoded one complete message correctly'. You decode one message and return 'OK'. DemuxingProtocolCodecFactory calls doDecode once more to decode remaining buffer. You return two more OKs decoding two message and it will cause the internal implementation return 'true' so that the loop continue. Now you've got half message left which is incomplete. DemusingProtocolCodecFactory calls once again if your decoder can decode the remaining data. You find out you cannot decode the half message because you need the whole message, and return NEED_DATA. NEED_DATA will be translated into 'return false', and the loop ends. doDecode will be called later when more data is received.

Do you understand what's going on? Please leave some comment. I'll add this documentation to JavaDoc if you think it is OK. Please confirm the JavaDoc and then close this issue.

Thanks,
Trustin

[Alex]

In my case buffer has only one message.
When decode is called I decode that message (whole buffer) and return OK.
Then doDecode will call decode again, althought it is not necessary since whole buffer was already processed! So, I don't like that my decode is unnecessarly called twice. Do you mean that I should implement decode in such way that it will check if buffer is fully processed and return NEED_DATA? or if buffer has incomplete message or even EMPTY message I should return NEED_DATA?

Alex

[Trustin Lee]

Sorry for the late reply.

I checked in the fix. doDecode() should not be called if buffer.hasRemaining() is false. Could you please retry with new snapshot? I updated maven repository, so you can simply use mina-0.7.2-SNAPSHOT.jar.

Thanks,
Trustin

[Alex]

This fix helped. Now I can send messages but only for awhile.

If I start sending them fast I still get that exception, sometimes on client, sometimes on server.

java.lang.IllegalStateException: doDecode() can't return true when buffer is not consumed.
        at org.apache.mina.protocol.codec.CumulativeProtocolDecoder.decode(CumulativeProtocolDecoder.java:109)
        at org.apache.mina.protocol.io.IoAdapter$SessionHandlerAdapter.dataRead(IoAdapter.java:135)
        at org.apache.mina.io.AbstractIoFilterChain$2.dataRead(AbstractIoFilterChain.java:150)
        at org.apache.mina.io.AbstractIoFilterChain.callNextDataRead(AbstractIoFilterChain.java:364)

If my decode returns NEED_DATA it works fine no matter how fast client sends messages.

Alex

[Trustin Lee]

This exception is thrown when you return true but not reading any byte. You have to read at least one byte if you returned true. Could you check this case in your decoder using a debugger or System.out.println()?

[Alex]

Exactly. My decode always returns OK. I don't know why but it was called twice and then there is nothing to read but still I return OK.

Let's do it in another way. You tell me what is wrong in my decode.
I send the simplified version of my decode method.

public MessageDecoderResult decode(ProtocolSession session, ByteBuffer in, ProtocolDecoderOutput out)
                                   throws ProtocolViolationException {
    long l = in.getLong();
    //out.write(msg);
    return MessageDecoder.OK;
}

And let's assume that buffer has only one long.
Is there anything wrong in this decode?

[Trustin Lee]

You have to check the remaining number of bytes before you read:

public MessageDecoderResult decode(ProtocolSession session, ByteBuffer in, ProtocolDecoderOutput out)
                                   throws ProtocolViolationException {
    if( in.remaining() < 8 )
        return MessageDecoder.NEED_DATA;

    long l = in.getLong();
    return MessageDecoder.OK;
}

BTW your code above will throw IndexOutOfBoundsException instead of IllegalStateException because it 'always' calls 'getLong()'. It seems like you simplified the code too much.

Thanks,
Trustin

[Alex]

That's what I was trying to say: we need to do this kind of checking (remaining()<8). That's fine. BTW, probably it is better to move it to decodable().

Still, something goes wrong. After awhile when buffer has more than one message I get that exception: doDecode() can't return true when buffer is not consumed.

I don't understand how I get it because in every decode() I read more than one byte.
I print the buffer on the entry and exit of my decode() methods.

I can see like this:
entry - java.nio.DirectByteBuffer[pos=0 lim=52 cap=64]
exit - java.nio.DirectByteBuffer[pos=13 lim=52 cap=64]

So, pos has changed, why does it still give me that exception?

Alex

[Trustin Lee]

Sorry for a late reply.

Adam gave me a clue so that I can fix this issue finally. Could you please retry with the recent snapshot?

Thanks,
Trustin

[Alex]

Now it works fine. Thanks!

Alex

[Trustin Lee]

OK, then could you close this issue? Thanks in advance!

[Alex]

Fixed.