Selector.select() starts an infinite loop

Question

I have a minimal JMS provider, which sends topic messages over UDP and queue messages over TCP. I use a single selector to handle UDP and TCP selection keys (registering both SocketChannels and DatagramChannels).

My problem is: if I only send and receive UDP packets, everything goes well, but as soon as I start writing on a TCP socket (using Selector.wakeup() to have the selector do the actual writing), the selector enters an infinite loop, returning an empty selection key set, and eating 100% CPU.

The code of the main loop (somewhat simplified) is:

public void run() {
  while (!isInterrupted()) {
   try {
    selector.select();
   } catch (final IOException ex) {
    break;
   }

  final Iterator<SelectionKey> selKeys = selector.selectedKeys().iterator();
  while (selKeys.hasNext()) {
    final SelectionKey key = selKeys.next();
    selKeys.remove();
    if (key.isValid()) {
     if (key.isReadable()) {
      this.read(key);
     }
     if (key.isConnectable()) {
      this.connect(key);
     }
     if (key.isAcceptable()) {
      this.accept(key);
     }
     if (key.isWritable()) {
      this.write(key);
      key.cancel();
     }
    }
   }
   synchronized(waitingToWrite) {
    for (final SelectableChannel channel: waitingToWrite) {
     try {
      channel.register(selector, SelectionKey.OP_WRITE);
     } catch (ClosedChannelException ex) {
      // TODO: reopen
     }
    }
    waitingToWrite.clear();
   }
  }
 }

And for a UDP send (TCP send is similar):

public void udpSend(final String xmlString) throws IOException {
  synchronized(outbox) {
    outbox.add(xmlString);
  }
  synchronized(waitingToWrite) {
    waitingToWrite.add(dataOutChannel);
  }
  selector.wakeup();
}

So, what's wrong here? Should I use 2 different selectors to handle UDP and TCP packets?

Answer 1

Problem went away after upgrading to Java 1.6.0_22.

Answer 2

I suggest you check the return value of select() method.

try {
 if(selector.select() == 0) continue;
} catch (final IOException ex) {
 break;
}

Did you try debugging to see where the loop is?

Edit:

• I recomend that instead of calling "remove()" on the iterator, you call selectedKeys.clear() after you iterate over them. It is possible that the implementation of the iterator, does not remove it from the underlying set.
• Check that you are not registering OP_CONNECT on a connected channel.

Answer 3

You are probably getting an IOException and ignoring it in the empty catch block. Never do that. And just continuing after an IOException is practically never the correct action. The only exception to that rule I can think of offhand is a SocketTimeoutException, and you're in non-blocking mode so you won't be getting those, and you don't get them on selectors anyway. I would want to see the content of your methods that handle connect, accept, read, and write.

Answer 4

Modify your design to have one thread per incoming network connection.

The selector should be used when you're using one thread to process incoming messages on multiple TCP sockets. You register each socket with the selector and then select(), which blocks until there is data available on one of them. Then you loop through each key and process the waiting data. This is the method I've always used when writing C code, and it will work, but I don't think it's the best way to do it in Java.

Java has great native thread support, which C does not. I think it makes a lot more sense to have one thread per TCP socket and not to use selectors at all. If you just do a read operation on a socket, the thread will block until data arrives or the socket is closed. This is effectively the same thing as selecting with only one registered channel.

If you want to get this to work with only one thread, you should use the selector only for TCP sockets where you want incoming connections. This way, the only time the call to select() will return is when there is incoming data waiting on one of the sockets. That thread will be asleep at all other times, and no other operation will wake it up.