garbled data passed in cumulation buffer for decode(), probably when data re-transmission.

[Trustin Lee (이희승)]

Hello Huican,

First of all, sorry for a very late response.

On Sat, Sep 12, 2009 at 2:41 PM, huican ping <pinghuican at gmail.com> wrote:
> Hello Trustin,
>
> I have been looking at that garbled channel buffer for my FrameDecoder
> case in these days.
> FYI, my dummy frameDecoder for comming message with structure (for
> example): StartByte+Any-Bytes-Different-Than-StartByte-Trailer+TrailerByte.
>
> So far, I saw both the two failed cases for my 1066 bytes (results in
> 2 messageReceived calls).
>  a) good-1024-bytes + bad data.   // Since I am waiting on the tailer
> byte, so it receiveTimeout.
>  b) bad-1024-bytes + good data.   // Since I found the trailer byte,
> but there is no good startByte, so it is bad message.
>
> 1:) I captured and tcpdumped one failed case inside
> million-and-million successful oens, and noticed that the client sent
> in a good message to the server based on the FrameDecoder.
> 2:) and when I read the code at: FrameDecoder.java, method messageReceived().
> Since the client can sends in a message which results in multiple
> messageReceived Call (such as 2 for my input), I just wonder how the
> code below to keep synchronizing the cumulation buffer for these if
> and else cases. What I am worrying is that when the previous
> messageReceived event at the else branch (not done yet), another
> messageReceived event can go into the if branch.
>
> Is this a problem?
>
> Thanks
> Huican Ping
>
> ===============
>    public void messageReceived(
>            ChannelHandlerContext ctx, MessageEvent e) throws Exception {
>
>        Object m = e.getMessage();
>        if (!(m instanceof ChannelBuffer)) {
>            ctx.sendUpstream(e);
>            return;
>        }
>
>        ChannelBuffer input = (ChannelBuffer) m;
>        if (!input.readable()) {
>            return;
>        }
>
>        ChannelBuffer cumulation = cumulation(ctx);
>        if (cumulation.readable()) {
>            cumulation.discardReadBytes();
>            cumulation.writeBytes(input);
>            callDecode(ctx, e.getChannel(), cumulation, e.getRemoteAddress());
>        } else {
>            callDecode(ctx, e.getChannel(), input, e.getRemoteAddress());
>            if (input.readable()) {
>                cumulation.writeBytes(input);
>            }
>        }
>    }
> ==========================

It will be a problem if messageReceived() is called by different
threads at the same time.  However, it does not happen because the
decoder is places before ExecutionHandler or ExecutionHandler uses
OrderedMemoryAwareThreadPoolExecutor.  If other executor is used and
the decoder is placed after the ExecutionHandler, FrameDecoder will
surely fail, but the failure should be very easy to reproduce.

> On Thu, Sep 10, 2009 at 10:17 PM, huican ping <pinghuican at gmail.com> wrote:
>> Hello Trustin,
>>
>> I thought I found some place in my parser suspicious, now my logic is
>> pretty much same as the example posted at the FrameDecoder page, and
>> still get the same issue.
>>
>> // My decode() logic.
>> protected Object decode(ChannelHandlerContext ctx, Channel channel,
>>                        ChannelBuffer buffer) throws Exception {
>>
>>        // return if the buffer empty
>>        if (buffer.readableBytes() <= 0) {
>>                return null;
>>        }
>>
>>        // Mark the current buffer position before parsing the buffer
>>        // because the whole message might not be in the buffer yet.
>>        // We will reset the buffer position to the marked position in
>>        // that case.
>>        buffer.markReaderIndex();
>>
>>
>>        for ( loc = buffer.readerIndex(); loc < buffer.writerIndex(); loc++)
>>        {
>>                // Paser code.
>>                // if find the whole message, then return that message
>>                // otherwise, it will hit the end of the loop
>>                // Inside the loop
>>
>>                // I didn't modify the buffer except change its readerIndex if
>>                // the whole message is not there yet.
>>        }
>>
>>
>>        // The whole message were not received yet - return null.
>>        // This method will be invoked again when more packets are
>>        // received and appended to the buffer.
>>
>>        // Reset to the marked position to read the length field again
>>        // next time.
>>        buffer.resetReaderIndex();
>>        return null;
>> }
>>
>> // my getPipeline() logic:
>>                pipeline.addLast("decoder", new acmeDecoder());
>>                pipeline.addLast("encoder", new acmeEncoder());
>>                pipeline.addLast("handler", new acmeServerHandler());
>>
>>
>> Thanks
>> Huican Ping
>>
>> On Thu, Sep 10, 2009 at 9:49 PM, Huican Ping <pinghuican at gmail.com> wrote:
>>>
>>> Hello Trustin,
>>>
>>> Probably it is my parser problem. Don't waste time on it now. I am
>>> sorry about it.
>>>
>>> Thanks
>>> Huican Ping
>>>
>>> On Thu, Sep 10, 2009 at 5:40 PM, Huican Ping (via Nabble)
>>> <ml-user+135163-919750110 at n2.nabble.com> wrote:
>>>> FYI, my decoder inherited from the FrameDecoder.
>>>>
>>>> Thanks
>>>> Huican Ping
>>
>
> _______________________________________________
> netty-users mailing list
> netty-users at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/netty-users
>

— Trustin Lee, http://gleamynode.net/