No problem Emmanuel, thanks for looking into this now! Yes I can post a JIRA bug, I just want to make sure I have the whole picture clear first.
As I look at it we are talking about two problems with the same origin - an Exception in a decode()-method. The first is that the DemuxingProtocolDecoder's state.currentDecoder is not cleared after an Exception is thrown. So any following received messages are passed directly to the previously used MessageDecoder's decode()-method without selecting a new decoder, using decodable()-calls. This differs from what happens if you return MessageDecoder.NOT_OK even though both are documented as protocol specification violations. You could go around this by catching all exceptions and return NOT_OK but as you can't both return and re-throw the exception, the nice ExceptionHandler-functionality in MINA goes to waste. I would rather suggest that the DemuxingProtocolDecoder in it's doDecode() catches Exceptions, resets the current decoder and re-throws the Exception. But I'm no expert of MINA internals. The second is that the buffer can be "half read" when the Exception is thrown and must some how be reset before passed to a new decoder if a new message is received. Otherwise the decoding will fail for sure again and we will probably end up with a new Exception and be stuck in a loop. And even if nothing was read from the buffer when the Exception is thrown, chances are big that a new decoding attempt of that same buffer will fail in the same way. Making the removeSessionBuffer() method accessible sounds nice but still forces the framework users to do a try-catch and re-throw in every decoder. Also I don't know an easy way to access the DemuxingProtocolDecoder instance from the MessageDecoder.decode()-method. Maybe a catch block in the DemuxingProtocolDecoder as I suggested for problem #1 could fix this also? Should I post one or two bugs in JIRA? /Björn -----Ursprungligt meddelande----- Från: Emmanuel Lecharny [mailto:[email protected]] Skickat: den 18 november 2010 12:54 Till: [email protected] Ämne: Re: VB: Question about DemuxingProtocolCodecFactory and MessageDecoder On 11/18/10 8:40 AM, Björn Thelberg wrote: > Did anyone have some input on this one? > > In short my problem is that on exceptions in a MessageDecoder's > decode()-method the DemuxingProtocolDecoder does not reset the > state.currentDecoder so following decoding operations (following messages) go > directly to the same decoder (does not try other decoders decodable()-method) > and therefore usually crash again. Se previous messages below for details. Sorry for the late answer, was busy with so many other issues that I simply skipped the thread ... You are basically right : when an exception occurs, it would be great to be able to reset the buffer instead of depending on the user's application to do so. The buffer containing the data is stored into the session's attribute, but as its name is built based on an instance hash value, we can't simply remove it from the session. However, it starts with "CumulativeProtocolDecoder.buffer@" (the class name may be different if you extended the CumulativeProtocolDecoder class, not sure though) followed by the hex value of the AttributeKey instance. Pfeww... One dirty workaround would be t get the list of Attributes stored into the session, and search for the one that starts with "CumulativeProtocolDecoder.buffer@", and remove it. Yuk :/ Way better would be to add a method allowing the application to clean the buffer (we already have a private method called removeSessionBuffer() which can be made public), or to add a method to process the ExceptionCaught event, and clean the buffer in this case (conditionally, of course : that means we should have a way to tell the filter to do that when needed only). Can I ask you to fill a JIRA with a description of this problem, so that we can discuss the solution and don't forget about it ? I'd like to have such a feature added for 2.0.2. Thanks ! > /Björn > > -----Ursprungligt meddelande----- > Från: Björn Thelberg [mailto:[email protected]] > Skickat: den 2 november 2010 18:23 > Till: [email protected] > Ämne: SV: Question about DemuxingProtocolCodecFactory and MessageDecoder > > Thanks for your reply Emmanuel! > > How do you mean I should "clean up" the session? > > I have tried to do the following in my decoder: > try { > //...decoding stuff > } catch (Exception e) { > in.skip(in.remaining()); > throw e; > } > It should fix the problem with remaining bytes in the buffer but I am still > stuck with the same decoders decodable()-call in the next response i receive > (no matter type). > > I see that the DemuxingProtocolDecoder does the following magic in doDecode(): > //... > else if (result == MessageDecoder.NOT_OK) { > state.currentDecoder = null; > //... > But exceptions are not catched in the doDecode(). > > Would it not make sense if the DemuxingProtocolDecoder did the same on > exceptions and re-threw them? > Just like NOT_OK results Exceptions are also documented as ""protocol > specification violation" in the MessageDecoder.decode(). > Or is there a use case that I am missing where you do want to return to the > same decoder after an exception? > > Even if there is a way to reset the current decoder from within a catch-block > in a decoder's decodable() that I am not aware of I think it would be nice if > you didn't have to do try-catch in every decoder implementation but rather > let the framework take care of that. Do you agree? > > /Björn Thelberg > > > -----Ursprungligt meddelande----- > Från: Emmanuel Lecharny [mailto:[email protected]] > Skickat: den 29 oktober 2010 12:17 > Till: [email protected] > Ämne: Re: Question about DemuxingProtocolCodecFactory and MessageDecoder > > On 10/28/10 1:34 PM, Björn Thelberg wrote: >> Hello > Hi, > >> I am writing a client implementation for a binary protocol using a >> ProtocolCodecFilter with a DemuxingProtocolCodecFactory and a list of >> MessageDecoder's in the same way as the Mina SumUp example. >> I also use a DemuxingIoHandler to handle the responses and I am in general >> very pleased with how this works. > Fine ! >> My question is as follows. >> If I for some reason get an exception in one of my MessageDecoder's >> decode()-method due to a broken response of some sort, what is supposed to >> happen next? > The ExceptionCaught event is generated, and you can handle it in your > handler. >> What happens for me is that the exception is passed to my ExceptionHandler >> and decoding of this message quits. Fine so far. > Ok, works as expected :) >> But the subsequent decode operation (following a new request) is immediately >> passed on to the same MessageDecoder instance's decode(). > Makes sense. I guess it's on the same session. >> This is of course a problem if the next response is of another type because >> the chain of MessageDecoder's decodable() are never called. >> Also it sometimes seems as if remaining bytes in the IoBuffer from >> decoding#1 (that threw Exception) are still in the buffer in decoding#2 and >> my decoding crashes (again). > You may have to clenup the session when you get the exception in your > decoder, otherwise the cumulative buffer will keep the faulty data. >> I interpret the Javadoc for MessageDecoder.decode() as if throwing exception >> would mean roughly the same thing as returning MessageDecoderResult.NOT_OK. >> Do they not both indicate a "protocol specification violation"? >> It seems as throwing exception in the decode()-method acts more like >> returning MessageDecoderResult.NEED_DATA. >> >> I quess I can solve this problem by catching all exceptions in decode() and >> returning MessageDecoderResult.NOT_OK but then my ExceptionHandler is never >> called. > You can also try to catch the exception, clean the session, and rethrow > the initial exception ? > > Let me know if it works. > > -- Regards, Cordialement, Emmanuel Lécharny www.iktek.com
