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
