Java NIO server Selector loop

Question

I have a server that uses Java NIO in non-blocking mode. Right now I'm using a single thread for the selector, and four worker threads to process the bytes after the selector reads them in. I'm going to switch to a thread pool soon enough. Anyway, that's not why I'm here.

Take a look at the code below. I have a state object of type CommunicationStatus called commsState which maintains various parameters relating to the SocketChannel, the number of bytes read, the current working state, whether the message is complete or not, the ByteBuffer for that particular SocketChannel, etc. This works well. The CommunicationStatus object is attached to the SelectionKey when the SocketChannel is registered with the Selector.

When I'm checking key.readyOps() to check what state the channel is in, I have an additional check within the OP_READ and OP_WRITE checks to make sure that the worker threads have finished processing the work before additional bytes are read from or written to the SocketChannel.
So for example, if ((key.readyOps() & SelectionKey.OP_READ) == SelectionKey.OP_READ) returns true but if (commsState.getCurrentState() == CommunicationStatus.STATE_READ) returns false then no bytes will be read at this time.
I find this works really well, but is it the correct way to do things? Is there a better way I could handle this?
Any other comments or thoughts are appreciated too.

private boolean running;
private ServerSocketChannel serverSocketChannel;

@Override
public void run() {

    try {

        Selector selector = Selector.open();
        serverSocketChannel = ServerSocketChannel.open();
        serverSocketChannel.configureBlocking(false);
        ServerSocket serverSocket = serverSocketChannel.socket();
        serverSocket.bind(new InetSocketAddress(serverPort));

        log.info("Selector Thread: FirmwareServer " + FirmwareServer.SERVER_VERSION
                + " Runnable- Listening for connections on port: " + serverSocket.getLocalPort());

        running = true;

        @SuppressWarnings("unused")
        SelectionKey serverAcceptKey = serverSocketChannel.register(selector, SelectionKey.OP_ACCEPT);

        while (running) {

            try {

                selector.select();
                Set<SelectionKey> selectedKeys = selector.selectedKeys();
                Iterator<SelectionKey> keyIterator = selectedKeys.iterator();

                while (keyIterator.hasNext()) {

                    SelectionKey key = (SelectionKey) keyIterator.next();
                    keyIterator.remove();

                    if (!key.isValid()) {
                        continue;
                    }
                    CommunicationStatus commsState = (CommunicationStatus) key.attachment();

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

                        acceptConnection(selector, key);

                    } else if ((key.readyOps() & SelectionKey.OP_READ) == SelectionKey.OP_READ) {

                        if (commsState.getCurrentState() == CommunicationStatus.STATE_READ) {

                            commsState.setSelectionKey(key);
                            readFromSocketChannel(key);

                        }
                    } else if ((key.readyOps() & SelectionKey.OP_WRITE) == SelectionKey.OP_WRITE) {

                        if (commsState.getCurrentState() == CommunicationStatus.STATE_WRITE) {

                            commsState.setSelectionKey(key);
                            writeToSocketChannel(key);

                        }
                    }
                }

            } catch (IOException e) {
                log.error("Selector IOException Occurred", e);
            }
        }

    } catch (IOException e) {

        log.error("Firmware Selector Thread: An IOException occurred", e);
        System.exit(-1);

    }

}

Answer 1

            Iterator<SelectionKey> keyIterator = selectedKeys.iterator();

            while (keyIterator.hasNext()) {

                SelectionKey key = (SelectionKey) keyIterator.next();

This cast here is not needed; you already know that you're going to get SelectionKey out of the keyiterator.

Looking up a bit, you declare Set<SelectionKey> selectedKeys = selector.selectedKeys();, but only use it for the iterator. The variable name doesn't add anything to understanding the code, and you might just want to say Iterator<SelectionKey> keyIterator = selector.selectedKeys().iterator();

---

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

This sort of statement, I'd wrap in a function. Bit manipulation tends to be very low level stuff and looks out of place in a function responsible for doing high level stuff.