Not getting OP_READ in my client

Question

I am new to NIO and NIO2. I have been playing with an Echo server sample code, which works flawlessly. I started to write a similar client. My goal is to have multiple client socket connections running off the one main-thread.

I do get the OP_CONNECT, then after that the selector does not return and times out from the: while(Selector.select(10000)>0) { .... }

If I ignore the selector and start reading the data with socketChannel.read(...), I can read the data. So, the data is ready to be read, but I just do not get Selector.select(10000) to return with some keys.

Here is the complete source code and I really would appreciate any help:

package com.maker.webscraping.nio;

import java.io.IOException;
import java.net.StandardSocketOptions;
import java.nio.ByteBuffer;
import java.nio.CharBuffer;
import java.nio.channels.SelectionKey;
import java.nio.channels.Selector;
import java.nio.channels.SocketChannel;
import java.nio.charset.Charset;
import java.nio.charset.CharsetDecoder;
import java.util.Iterator;
import java.util.Map;
import java.util.Random;
import java.util.concurrent.ConcurrentHashMap;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;


public class EchoClient2 {
    private static final Logger logger = LoggerFactory.getLogger(EchoClient2.class);
    private static final Map<SocketChannel, EchoClient2> mapClients = new ConcurrentHashMap<>();

    private static final int DEFAULT_PORT = 5555;
    private static final String IP = "127.0.0.1";

    private final int clientID;
    private final ByteBuffer buffer;
    private final SocketChannel socketChannel;
    private final CharsetDecoder decoder;

    public int getClientID() {
        return clientID;
    }

    public ByteBuffer getBuffer() {
        return buffer;
    }

    public CharsetDecoder getDecoder() {
        return decoder;
    }

    //private static Selector selector = null;
    public static void main(String[] args) {
        Selector selector = null;
        try {
            selector = Selector.open();
            if (!selector.isOpen()) 
                throw new RuntimeException("Selector closed!");

            EchoClient2[] clients = new EchoClient2[2];

            clients[0] = new EchoClient2(0, selector);

            // wait for incomming events
            while (selector.select(10000)>0) {
                Iterator<SelectionKey> keys = selector.selectedKeys().iterator();
                while (keys.hasNext()) {
                    SelectionKey key = (SelectionKey) keys.next();

                    try (SocketChannel socketChannel = (SocketChannel) key.channel()) {

                        if (key.isConnectable()) {
                            // connected
                            logger.info("Client:{} connected!", clients[0].getClientID());
                            key.interestOps(SelectionKey.OP_READ); // <-- desprete tries
                            socketChannel.register(selector, SelectionKey.OP_READ | SelectionKey.OP_WRITE); // <-- desprete tries
                            logger.info("R:{}, W:{}, C:{}, validOps:{}", SelectionKey.OP_READ, SelectionKey.OP_WRITE, SelectionKey.OP_CONNECT,  socketChannel.validOps());
                            // close pending connections
                            if (socketChannel.isConnectionPending()) {
                                socketChannel.finishConnect();
                            }

                            read(key,selector); // <-- desprete tries
                            if (key.isReadable()) {
                                read(key,selector);
                            }// else if (key.isWritable()) {
                            //  this.writeOP(key);
                            //}

                        }
                    } catch (IOException e) {
                        logger.error("SocketChannel Exception!", e);
                    }
                }
            }
        } catch (IOException e) {
            logger.error("Selector IOException!", e);
        } finally {
            try {
                selector.close();
            } catch (IOException e) {}
        }       
    }

    public EchoClient2(int clientID, Selector selector) throws IOException {
        this.clientID = clientID;
        buffer = ByteBuffer.allocateDirect(2 * 1024);

        Charset charset = Charset.defaultCharset();
        decoder = charset.newDecoder();
//      if (selector==null)
//          selector = Selector.open();
        socketChannel = SocketChannel.open();
        if ((socketChannel.isOpen()) && (selector.isOpen())) {

            // configure non-blocking mode
            socketChannel.configureBlocking(false);
            // set some options
            socketChannel.setOption(StandardSocketOptions.SO_RCVBUF,
                    128 * 1024);
            socketChannel.setOption(StandardSocketOptions.SO_SNDBUF,
                    128 * 1024);
            socketChannel.setOption(StandardSocketOptions.SO_KEEPALIVE,
                    true);

            socketChannel.register(selector, SelectionKey.OP_CONNECT | SelectionKey.OP_READ | SelectionKey.OP_WRITE);
            //socketChannel.register(selector, SelectionKey.OP_CONNECT);

            // connect to remote host
            socketChannel.connect(new java.net.InetSocketAddress(IP, DEFAULT_PORT));

            // add it to the map
            mapClients.put(socketChannel, this);
        } else 
            throw new RuntimeException("Channel or Selector closed!");
    }

    // isReadable returned true
    private static int read(SelectionKey key, Selector selector) {

        try {
            SocketChannel socketChannel = (SocketChannel) key.channel();
            EchoClient2 client = mapClients.get(socketChannel);
            ByteBuffer buffer = client.getBuffer();

            buffer.clear();
            int numRead = -1;
            try {
                numRead = socketChannel.read(buffer);
            } catch (IOException e) {
                System.err.println("Cannot read error!");
            }

            if (numRead == -1) {
                mapClients.remove(socketChannel);
                System.out.println("Connection closed by: " + socketChannel.getRemoteAddress());
                socketChannel.close();
                key.cancel();
                return 1;
            }

            if (numRead == 0)
                throw new RuntimeException("numRead is 0!!!");

            buffer.flip();

            CharBuffer charBuffer = client.getDecoder().decode(buffer);
            System.out.println("server says:" + charBuffer.toString());

            if (buffer.hasRemaining()) {
                buffer.compact();
            } else {
                buffer.clear();
            }

            int r = new Random().nextInt(100);
            //if (r == 50) {
            //  System.out.println("50 was generated! Close the socket channel!");
            //  return 1;
            //} 

            ByteBuffer randomBuffer = ByteBuffer.wrap("Random number:".concat(String.valueOf(r))
                            .getBytes("UTF-8"));
            socketChannel.write(randomBuffer);
            key.interestOps(SelectionKey.OP_READ | SelectionKey.OP_WRITE);
            socketChannel.register(selector, SelectionKey.OP_READ | SelectionKey.OP_WRITE); // <-- desprete tries
        } catch (IOException e) {
            logger.error("IOException inside read!", e);

        } 
        return 0;
    }
}

Thanks,

Answer 1

while (selector.select(10000)>0)

This code is already wrong. It will stop selecting the first time a select timeout occurs. It should be:

while (selector.isOpen())
{
    if (selector.select(10000) > 0)
    {
        // ...
    }
}

There are other problems.

1. You should only assume the connection is complete if finishConnect() returns true.
2. Reading when the connection is complete isn't valid. You should only read when key.isReadable() is true. You need a separate test for that, not one piggybacked onto the isConnectable() case.
3. Similarly you need a separate case for isWritable().
4. You shouldn't register OP_WRITE until you have something to write, or better still after you get a zero-length return when you try to write something. OP_WRITE signals that the socket send buffer isn't full. It is almost always true.
5. You need to call keys.remove() after keys.next(), otherwise you will keep getting the same selected keys over and over again. The selector doesn't clear the selected-key set.
6. Your comment // close pending connections is completely incorrect.
7. The following isConnectionPending() call is redundant. Of course it's pending, that's why you got the OP_CONNECT.