mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Emmanuel Lecharny (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DIRMINA-1013) Threading model is supressed by ProtocolCodecFilter
Date Thu, 04 Jun 2015 23:14:38 GMT

    [ https://issues.apache.org/jira/browse/DIRMINA-1013?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14573763#comment-14573763
] 

Emmanuel Lecharny commented on DIRMINA-1013:
--------------------------------------------

My bad ! I was thinking 'session', not 'decoder'. 

There are a few thing to take care of :
* a decoder might be stateful, thus keeping a state for further decoding. In this case, we
must keep such decoder instance associated to a session, not to a thread (or a IoProcessor).
* if the decoder is stateless, then we don't need any lock, as we wont have any possible interaction
with any other thread.

Actually, I don't see why we do protect the decoding at all in the {{ProtocolCodecFilter}},
it should be the codec responsability to protect itself against concurrent actions.

> Threading model is supressed by ProtocolCodecFilter
> ---------------------------------------------------
>
>                 Key: DIRMINA-1013
>                 URL: https://issues.apache.org/jira/browse/DIRMINA-1013
>             Project: MINA
>          Issue Type: Bug
>          Components: Core, Filter
>    Affects Versions: 2.0.9
>         Environment: Windows 7 x32 
> Java(TM) SE Runtime Environment (build 1.8.0_45-b14) 
> Java HotSpot(TM) Client VM (build 25.45-b02, mixed mode, sharing)
>            Reporter: Marat Gainullin
>
> ProtocolCodecFilter.messageReceived uses a semaphore to protect the following critical
section: 
> {code}
>                 lock.acquire();
>                 // Call the decoder with the read bytes
>                 decoder.decode(session, in, decoderOut);
>                 // Finish decoding if no exception was thrown.
>                 decoderOut.flush(nextFilter, session);
>                 ...
> {code}
> in such fragment of code:
> {code}
>         // Loop until we don't have anymore byte in the buffer,
>         // or until the decoder throws an unrecoverable exception or
>         // can't decoder a message, because there are not enough
>         // data in the buffer
>         while (in.hasRemaining()) {
>             int oldPos = in.position();
>             try {
>                 lock.acquire();
>                 // Call the decoder with the read bytes
>                 decoder.decode(session, in, decoderOut);
>                 // Finish decoding if no exception was thrown.
>                 decoderOut.flush(nextFilter, session);
>             } catch (Exception e) {
>                 ProtocolDecoderException pde;
>                 if (e instanceof ProtocolDecoderException) {
>                     pde = (ProtocolDecoderException) e;
>                 } else {
>                     pde = new ProtocolDecoderException(e);
>                 }
>                 if (pde.getHexdump() == null) {
>                     // Generate a message hex dump
>                     int curPos = in.position();
>                     in.position(oldPos);
>                     pde.setHexdump(in.getHexDump());
>                     in.position(curPos);
>                 }
>                 // Fire the exceptionCaught event.
>                 decoderOut.flush(nextFilter, session);
>                 nextFilter.exceptionCaught(session, pde);
>                 // Retry only if the type of the caught exception is
>                 // recoverable and the buffer position has changed.
>                 // We check buffer position additionally to prevent an
>                 // infinite loop.
>                 if (!(e instanceof RecoverableProtocolDecoderException) || (in.position()
== oldPos)) {
>                     break;
>                 }
>             } finally {
>                 lock.release();
>             }
>         }
> {code}
> Using of semaphore
> {code}
> public class ProtocolCodecFilter extends IoFilterAdapter {
> ...
>     private final Semaphore lock = new Semaphore(1, true);
> {code}
> pushs other threads to wait while one of them is decoding. In MINA 2.0.7 there was a
synchronized block at the same place, but on other point - decoderOut, wich is created per
ioSession. Thus it was a stripped lock.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message