Search code examples
javamultithreadingsocketsnio

Java NIO SeletionKey iterator and key handling, am I doing it right?


I'm following this tutorial on Java NIO. I found what appears to be the correct java file Here. I've adjusted my code to operate on a single port instead of multiple ports, and to simply output the data to the screen rather than echoing it back to the client.

When I run the code, both my version and the version on the hyper-link above, it appears that the channels aren't handled properly. I came to this conclusion simply from the sound of my laptop's fan, and after placing a few quick System.out.println() messages I managed to narrow the issue down a bit. At first I thought the selector.select() method wasn't blocking for some reason, the channels appeared to be empty, but the while(hasContent) loop kept iterating.

After a bit of searching I came across this post and realised that maybe I wasn't handling keys correctly. I adjusted my code to cancel the SelectionKey in the if statement at the bottom of my while(iterator.hasNext()) loop, once the data has been read from the channel.

This seems to have done the trick, now I get one message informing that there are no more bytes to read before the channel is closed, and things seem to run more smoothly overall.

Here is my code for what I've done so far, am I correct in my thinking about how this should be implemented? Am I cancelling the key at the correct point?

public class ServerRunnable implements Runnable {

    private int serverPort;
    private int queueLength;

    public ServerRunnable(int serverPort, int queueLength) {
        this.serverPort = serverPort;
        this.queueLength = queueLength;

    }

    private boolean running;
    private ServerSocketChannel serverSocketChannel;
    private ByteBuffer buffer = ByteBuffer.allocate(1024);

    @Override
    public void run() {
        try {
            Selector selector = Selector.open();

            serverSocketChannel = ServerSocketChannel.open();
            serverSocketChannel.configureBlocking(false);
            ServerSocket serverSocket = serverSocketChannel.socket();
            serverSocket.bind(new InetSocketAddress(serverPort));
            System.out
                    .println("DeviceServerV0.2 - Going to Listen for connections on port: "
                            + serverSocket.getLocalPort());
            running = true;

            SelectionKey serverAcceptKey = serverSocketChannel.register(
                    selector, SelectionKey.OP_ACCEPT);
            int count = 0;
            while (running) {
                int keyCount = selector.select();
                System.out.println("keyCount: " + keyCount);
                Set<SelectionKey> selectedKeys = selector.selectedKeys();
                Iterator keyIterator = selectedKeys.iterator();
                while (keyIterator.hasNext()) {
                    SelectionKey key = (SelectionKey) keyIterator.next();

                    if ((key.readyOps() & SelectionKey.OP_ACCEPT) == SelectionKey.OP_ACCEPT) {

                        ServerSocketChannel ssc = (ServerSocketChannel) key
                                .channel();
                        SocketChannel sc = ssc.accept();
                        sc.configureBlocking(false);

                        // Here we add the new connection to the selector
                        SelectionKey newKey = sc.register(selector,
                                SelectionKey.OP_READ);

                        keyIterator.remove();

                    } else if ((key.readyOps() & SelectionKey.OP_READ) == SelectionKey.OP_READ) {
                        SocketChannel sc = (SocketChannel) key.channel();

                        // Going with this from the tutorial for now.
                        // TODO: Implement proper boolean controls here
                        while (true) {
                            buffer.clear();

                            int read = sc.read(buffer);

                            if (read <= 0) {
                                // System.out.println("Bytes read: " + read);
                                System.out
                                        .println("No more bytes, breaking loop.");

                                break;
                            } else {

                                buffer.flip();
                                String result = new String(buffer.array());
                                System.out
                                        .println("Buffer Contents: " + result);

                            }
                        }
                        keyIterator.remove();
                        key.cancel();

                    }

                }

            }

        } catch (Exception e) {
            e.printStackTrace();
        }

    }

}

I'm also aware that calling ByteBuffer.array() doesn't effect the current position in the byteBuffer, so it would still appear as if the buffer hasn't been read. Should I handle this a certain way or is it enough to call ByteBuffer.clear() a the start of the loop?

I'm intending to move on to try build an NIO server that uses a pool of threads to handle requests, but I want to be sure my understanding of NIO is correct so far.


Solution

  • When a channel’s readyOps() contain SelectionKey.OP_READ it implies that some data can be read, not that the channel can be read entirely up to its end. But that’s what you are doing in your while(true) loop; you are reading and therefore polling a channel until its end has been reached.

    The correct way to handle it is to read once and do not attempt to read again, unless the next select reports that there is more data to read.