mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Emmanuel Lecharny <elecha...@gmail.com>
Subject PorotocolCodecFilter potential pbs and improvement
Date Wed, 03 Sep 2008 06:08:06 GMT
Hi,

while adding some Javadoc into this class, I have found that the 
encoder/decoder instances creation might be not thread safe. We are 
using a getDecoder0() where we have such a code :

    private ProtocolDecoder getDecoder0(IoSession session) throws 
Exception {
        ProtocolDecoder decoder = (ProtocolDecoder) session
                .getAttribute(DECODER);
        if (decoder == null) {
            decoder = factory.getDecoder(session);
            session.setAttribute(DECODER, decoder);
        }
        return decoder;
    }

This method is called for every messageReceived() invocation :

    public void messageReceived(NextFilter nextFilter, IoSession session,
            Object message) throws Exception {
        if (!(message instanceof IoBuffer)) {
            nextFilter.messageReceived(session, message);
            return;
        }

        IoBuffer in = (IoBuffer) message;
        ProtocolDecoder decoder = getDecoder0(session);
        ...

I know it's very unlikely that we receive two messages on the same 
session at the same time, but this can be a possibility, AFAIK. I 
suggest we synchronize this portion of the code this way :


    private ProtocolDecoder getDecoder0(IoSession session) throws 
Exception {
        // We have to synchronize this section as we may have to create 
the decoder
        // here on the first invocation.
        synchronized (factory) {
            ProtocolDecoder decoder = (ProtocolDecoder) 
session.getAttribute(DECODER);

            if ( decoder == null ) {
                // No existing instance ? Create it and stores it into 
the session
                decoder = factory.getDecoder(session);
                session.setAttribute(DECODER, decoder);
            }
           
            return decoder;
        }

Now, I think we can d something slightly better : we have two methods - 
getDecoder0(IoSession) and getDecoder(IoSession) - doing the exact same 
thing (returning the session decoder), except the fact that 
getDecoder0() inject the decoder into the session, if it was not already 
done. It makes me think that we might want to do taht during the 
createSession() event processing, instead of doing it while processing  
the first message received.

We will just have to implement the sessionCreated() method, and then we 
can remove the getDecoder0() method, using the getDecoder() method 
instead, without any synchronization.

thoughts ?

-- 
--
cordialement, regards,
Emmanuel L├ęcharny
www.iktek.com
directory.apache.org



Mime
View raw message