SelectionKey iterator.remove() throws UnsupportedOperationException and infinite loop

Question

I have a method that opens a connection, queries a site, gets the number of pages and then uses NIO to concurrently retrieve all of the pages. The first query is done using URLConnection and works perfectly fine. I am running in to 2 issues when I try to use NIO selectors and channels:

1) If I don't remove the key from the iterator, in runs in an infinite loop printing size() and sending queries. If I try to remove the key, I get an UnsupportedOperationsException. Bah!

2) Do I need to deregister the channel from OP_WRITE after I've written to the socket? If so, can I just call channel.register(selector, SelectionKey.OP_READ) to remove the interest in writing?

public void test() throws IOException {
// create selector
Selector selector = Selector.open();
System.out.println("opened");

// get the number of pages
URL itemUrl = new URL(ITEM_URL);
URLConnection conn = itemUrl.openConnection();
conn.setDoOutput(true);
OutputStreamWriter out = new OutputStreamWriter(conn.getOutputStream());
// out.write(getHeaderString(itemUrl));
out.write(new Query("", "Internal Hard Drives", false, false, true, false, -1, 7603, 1, 14, -1, "", "PRICE", 1).toString());
out.close();

JsonReader in = new JsonReader(new InputStreamReader(conn.getInputStream()));
JsonParser parser = new JsonParser();
JsonObject tempObj = (JsonObject) parser.parse(in);
Pages.setNumOfPages(getNumberOfIterations(tempObj.get("PaginationInfo")));
System.out.println("Pages: " + Pages.getNumOfPages());

    // for each page, create a channel, attach to selector with interest in read
    // typically this would be i <= Pages.getNumberOfPages but to troubleshoot, i'm limiting this to just once.
for (int i = 1; i <= 1; i++) {
    SocketChannel channel = SocketChannel.open();
    channel.configureBlocking(false);
    channel.connect(new InetSocketAddress(itemUrl.getHost(), 80));
    channel.register(selector, SelectionKey.OP_WRITE | SelectionKey.OP_READ);
}
selector.select();
Set<SelectionKey> sk = selector.keys();

while (!sk.isEmpty()) {
    System.out.println(sk.size());
    Iterator<SelectionKey> iterator = sk.iterator();
    while (iterator.hasNext()) {
    SelectionKey key = iterator.next();

            iterator.remove();
    if (key.isReadable()) {
        SocketChannel channel = (SocketChannel) key.channel();
        ByteBuffer buf = ByteBuffer.allocate(8192);
        channel.read(buf);
        buf.flip();
        Product p = parse(buf, Product.class);
        if (p != null) {
        finalItems.add(p);
        System.out.println("Item added!");
        key.cancel();
        }
    } else if (key.isWritable()) {
        SocketChannel channel = (SocketChannel) key.channel();
        System.out.println(itemUrl);
        System.out.println(new Query("", "Internal Hard Drives", false, false, true,
            false, -1, 7603, 1, 14, -1, "", "PRICE", 1).toString());
        channel.write(ByteBuffer.wrap(new Query("", "Internal Hard Drives", false,
            false, true, false, -1, 7603, 1, 14, -1, "", "PRICE", 1).toString()
            .getBytes()));

    }
    }
    selector.select();
    sk = selector.keys();
}
}

Answer 1

From http://docs.oracle.com/javase/6/docs/api/java/nio/channels/Selector.html#keys()

"The key set is not directly modifiable. A key is removed only after it has been cancelled and its channel has been deregistered. Any attempt to modify the key set will cause an UnsupportedOperationException to be thrown."

You want to be using Selector.selectedKeys();

"Keys may be removed from, but not directly added to, the selected-key set. Any attempt to add an object to the key set will cause an UnsupportedOperationException to be thrown."

selector.select();
Set<SelectionKey> sk = selector.selectedKeys();

And then you can use Iterator.remove()

A good example is posted here http://tutorials.jenkov.com/java-nio/selectors.html at the bottom of the page

Answer 2

To answer (2): You should only register OP_WRITE as follows:

1. You have data to write.
2. You do the write, looping until the buffer is empty or write() returns zero.
3. If write() returned zero, register the channel for OP_WRITE, remember the buffer somehow, and keep selecting.
4. When OP_WRITE fires, try the write again as above. This time, if it doesn't return zero, deregister OP_WRITE. You can now forget about that buffer too, if it was temporary.
5. Otherwise just keep selecting.

The reason you only use OP_WRITE like this is that is almost always ready (because there is almost always space in the socket send buffer), but you aren't almost always ready to write. So, registering OP_WRITE permanently just results in the selector returning immediately, which is useless.

Re-registering as per your last paragraph isn't the correct way to change what events the channel is registered for. It creates a new SelectionKey, and, as far as I know, doesn't cancel the old one. The correct way is to call readyOps() on the existing SelectionKey.