You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Emmanuel Lécharny <el...@gmail.com> on 2013/01/03 19:40:45 UTC

[MINA 3] Some issue with the way we manage new sessions if we have different threads

Hi !

I tried to inject the benchmark we have in MINA 2 into MINA3, using the
NioTcpClient Julien just added. It was easy to migrate the code, but
now, I have some issues...

Basically I get a NPE when trying to manipulate the session
selectionKey, which may become null in some case. This is typically a
race condition, where this SelectionKey is deregistered from one
selector (the Connect one) and registered on the IoLoop selector.

Let me explain what's going on...

I have a client that tries to write some data when teh session get opened :

            public void sessionOpened(IoSession session) {
                sendMessage(session, data);
            }

There is nothing wrong here : it's perfectly elicit to write some data
as soon as we are informed that the session is now connected. The
problem is that, in the middle, we have *moved* the channel from one
selector to the other, and in the middle of this process, we set the
SelectionKey to null :

In NioTcpSession :

    public void ready(final boolean accept, boolean connect, final
boolean read, final ByteBuffer readBuffer,
            final boolean write) {
        if (connect) {
                    // cancel current registration for connection
                    selectionKey.cancel();
                    selectionKey = null;  
<<<<----------------------------- Here !

                    // Register for reading
                    selectorLoop.register(false, false, true, false,
this, channel, null);

                    // and inform the IoHandler that the session is
connected
                    setConnected();

The setConnected() call will itself call the processSessionOpen(). If it
goes faster than the processing of the selectorLoop.register()
processing, then we will have a null selectionKey when we process the
NioSelectorLoop.modifyRegistration() method :

    public void modifyRegistration(final boolean accept, final boolean
read, final boolean write,
            final SelectorListener listener, final SelectableChannel
channel) {

        final SelectionKey key = channel.keyFor(selector);
        if (key == null) {
            logger.error("Trying to modify the registration of a not
registered channel");

The stack trace from the setConnected() call is :

Daemon Thread [SelectorWorker connect-0] (Suspended)   
    NioSelectorLoop.modifyRegistration(boolean, boolean, boolean,
SelectorListener, SelectableChannel) line: 142   
    NioTcpSession.flushWriteQueue() line: 225   
    NioTcpSession(AbstractIoSession).enqueueWriteRequest(Object) line:
553   
    NioTcpSession(AbstractIoSession).enqueueFinalWriteMessage(Object)
line: 868   
    NioTcpSession(AbstractIoSession).processMessageWriting(Object,
IoFuture<Void>) line: 793   
    NioTcpSession(AbstractIoSession).doWriteWithFuture(Object,
IoFuture<Void>) line: 523   
    NioTcpSession(AbstractIoSession).write(Object) line: 501   
    Mina3BenchmarkClient$1.sendMessage(IoSession, byte[]) line: 50   
    Mina3BenchmarkClient$1.sessionOpened(IoSession) line: 55   
    NioTcpSession(AbstractIoSession).processSessionOpen() line: 666   
    NioTcpSession.setConnected() line: 202   
    NioTcpSession.ready(boolean, boolean, boolean, ByteBuffer, boolean)
line: 398   
    NioSelectorLoop$SelectorWorker.run() line: 209   


Again, this is random and not too frequent, but I can assure that it
actually *happens*.

How could we avoid such a behavior ?

I think we should wait for the selection key to be set to its new value
here, which means we have to add some kind of synchronization between
the two threads (yuk :/), or transfer the setConnected() call to the
second thread (which is, IMO, the best thing to do).

What about adding a call back that does that in the ready() call ? Like in :


    public void ready(final boolean accept, boolean connect, final
boolean read, final ByteBuffer readBuffer,
            final boolean write) {
        if (connect) {
                    // cancel current registration for connection
                    selectionKey.cancel();
                    selectionKey = null;  
<<<<----------------------------- Here !

                    // Register for reading
                    selectorLoop.register(false, false, true, false,
this, channel, new RegistrationCallback() {

                        @Override
                        public void done(SelectionKey selectionKey) {
                            setConnected();
                        }
                    } );

Wdyt ?

PS : I will commit the benchmark module shortly, so that anyone can play
with the code and see what's going on, if it's not clear.

Thanks !

-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com 


Re: [MINA 3] Some issue with the way we manage new sessions if we have different threads

Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 1/3/13 7:40 PM, Emmanuel Lécharny a écrit :
> Hi !
>
> I tried to inject the benchmark we have in MINA 2 into MINA3, using the
> NioTcpClient Julien just added. It was easy to migrate the code, but
> now, I have some issues...
>
> Basically I get a NPE when trying to manipulate the session
> selectionKey, which may become null in some case. This is typically a
> race condition, where this SelectionKey is deregistered from one
> selector (the Connect one) and registered on the IoLoop selector.
>
> Let me explain what's going on...
>
> I have a client that tries to write some data when teh session get opened :
>
>             public void sessionOpened(IoSession session) {
>                 sendMessage(session, data);
>             }
>
> There is nothing wrong here : it's perfectly elicit to write some data
> as soon as we are informed that the session is now connected. The
> problem is that, in the middle, we have *moved* the channel from one
> selector to the other, and in the middle of this process, we set the
> SelectionKey to null :
>
> In NioTcpSession :
>
>     public void ready(final boolean accept, boolean connect, final
> boolean read, final ByteBuffer readBuffer,
>             final boolean write) {
>         if (connect) {
>                     // cancel current registration for connection
>                     selectionKey.cancel();
>                     selectionKey = null;  
> <<<<----------------------------- Here !
>
>                     // Register for reading
>                     selectorLoop.register(false, false, true, false,
> this, channel, null);
>
>                     // and inform the IoHandler that the session is
> connected
>                     setConnected();
>
> The setConnected() call will itself call the processSessionOpen(). If it
> goes faster than the processing of the selectorLoop.register()
> processing, then we will have a null selectionKey when we process the
> NioSelectorLoop.modifyRegistration() method :
>
>     public void modifyRegistration(final boolean accept, final boolean
> read, final boolean write,
>             final SelectorListener listener, final SelectableChannel
> channel) {
>
>         final SelectionKey key = channel.keyFor(selector);
>         if (key == null) {
>             logger.error("Trying to modify the registration of a not
> registered channel");
>
> The stack trace from the setConnected() call is :
>
> Daemon Thread [SelectorWorker connect-0] (Suspended)   
>     NioSelectorLoop.modifyRegistration(boolean, boolean, boolean,
> SelectorListener, SelectableChannel) line: 142   
>     NioTcpSession.flushWriteQueue() line: 225   
>     NioTcpSession(AbstractIoSession).enqueueWriteRequest(Object) line:
> 553   
>     NioTcpSession(AbstractIoSession).enqueueFinalWriteMessage(Object)
> line: 868   
>     NioTcpSession(AbstractIoSession).processMessageWriting(Object,
> IoFuture<Void>) line: 793   
>     NioTcpSession(AbstractIoSession).doWriteWithFuture(Object,
> IoFuture<Void>) line: 523   
>     NioTcpSession(AbstractIoSession).write(Object) line: 501   
>     Mina3BenchmarkClient$1.sendMessage(IoSession, byte[]) line: 50   
>     Mina3BenchmarkClient$1.sessionOpened(IoSession) line: 55   
>     NioTcpSession(AbstractIoSession).processSessionOpen() line: 666   
>     NioTcpSession.setConnected() line: 202   
>     NioTcpSession.ready(boolean, boolean, boolean, ByteBuffer, boolean)
> line: 398   
>     NioSelectorLoop$SelectorWorker.run() line: 209   
>
>
> Again, this is random and not too frequent, but I can assure that it
> actually *happens*.
>
> How could we avoid such a behavior ?
>
> I think we should wait for the selection key to be set to its new value
> here, which means we have to add some kind of synchronization between
> the two threads (yuk :/), or transfer the setConnected() call to the
> second thread (which is, IMO, the best thing to do).
>
> What about adding a call back that does that in the ready() call ? Like in :
>
>
>     public void ready(final boolean accept, boolean connect, final
> boolean read, final ByteBuffer readBuffer,
>             final boolean write) {
>         if (connect) {
>                     // cancel current registration for connection
>                     selectionKey.cancel();
>                     selectionKey = null;  
> <<<<----------------------------- Here !
>
>                     // Register for reading
>                     selectorLoop.register(false, false, true, false,
> this, channel, new RegistrationCallback() {
>
>                         @Override
>                         public void done(SelectionKey selectionKey) {
>                             setConnected();
>                         }
>                     } );
>
> Wdyt ?
>
> PS : I will commit the benchmark module shortly, so that anyone can play
> with the code and see what's going on, if it's not clear.
>
> Thanks !
>

Btw, the problem is exatcly the same for NioTcpServer : in the test I'm
running I get some NPE because the session 'state', which is established
in the sessionOpened() event, is sometime null when processing the
messageReceived() event, just because it's processed before the
sessionOpened() event. The fix seems to work too.

-- 
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com