Rare infinite loop decoding RTMP

[Joshua Warner]

Hi,

I'm seeing a problem on our red5 servers (using 1.0.0 RC 1) where the RtmpProtocolDecoder enters a fairly tight infinite loop, in the case where the connection is in the STATE_DISCONNECTING state and there's data remaining to be decoded.  This happens about once a month, I'd guess - so not super-often, but it forces us to restart our production servers at often inconvenient times.

Here's what I'm seeing when debugging the problem on the live server: the case for RTMP.STATE_DISCONNECTING isn't handled in the switch statement on RtmpProtocolDecoder.java:182.  This causes the decode(ProtocolState,IoBuffer) method to return without consuming any data from the IoBuffer.  See the relevant switch statement:

final RTMP rtmp = (RTMP) state;

switch (rtmp.getState()) {

case RTMP.STATE_CONNECTED:

case RTMP.STATE_DISCONNECTING:

return decodePacket(rtmp, in);

case RTMP.STATE_CONNECT:

case RTMP.STATE_HANDSHAKE:

return decodeHandshake(rtmp, in);

case RTMP.STATE_ERROR:

// attempt to correct error

default:  // <------ We're hitting this case, leaving data in the IoBuffer "in" unconsumed

return null;

}

The condition are such that the none of the break conditions in the while(true) loop on RtmpProtocolDecoder.java:112 to execute - buffer.remaining() is perpetually > 0, state.canStartDecoding(remaining) is true, and state.canContinueDecoding() is true.  Here's the loop:

while (true) {

final int remaining = buffer.remaining();

if (state.canStartDecoding(remaining)) {

state.startDecoding();

} else {

break;

}

final Object decodedObject = decode(state, buffer);

if (state.hasDecodedObject()) {

if (decodedObject != null) {

result.add(decodedObject);

}

} else if (state.canContinueDecoding()) {

continue;

} else {

break;

}

if (!buffer.hasRemaining()) {

break;

}

}

I'm sure that RTMP.STATE_DISCONNECTING should be handled differently in that switch statement than it is today.  Perhaps this also applies to RTMP.STATE_DISCONNECTED and RTMP.STATE_ERROR as well.

I can see two potential things we can do in those states to make sure we don't get in the infinite loop:

* Go ahead and just decode the rest of the data for that connection i.e:

case RTMP.STATE_CONNECTED:

case RTMP.STATE_DISCONNECTING:

                                // maybe: case RTMP.STATE_DISCONNECTED:

return decodePacket(rtmp, in);

* Discard the input data in these cases.  i.e.:

case RTMP.STATE_ERROR:

case RTMP.STATE_DISCONNECTING:

case RTMP.STATE_DISCONNECTED:

default:

// throw away any remaining input data:

in.position(in.limit());

return null;

Thoughts?

Thanks,

Joshua

[Mondain]

Thanks for looking into this issue Joshua, I appreciate the details for certain. I believe this option may be best for the core RTMP handling and I'll add it to see how it performs.

case RTMP.STATE_ERROR:

case RTMP.STATE_DISCONNECTING:

case RTMP.STATE_DISCONNECTED:

default:

// throw away any remaining input data:

in.position(in.limit());

return null;

Paul

--
 
---
You received this message because you are subscribed to the Google Groups "red5" group.
To unsubscribe from this group and stop receiving emails from it, send an email to red5interest...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

--
http://gregoire.org/
http://code.google.com/p/red5/