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
