Fix a bug where NioEventLoop.processSelectedKeys() enters an infinite loop when too many connections are closed at once

- Fixes #1171
This commit is contained in:
Trustin Lee 2013-03-18 15:01:19 +09:00
parent 97f2fa7341
commit f4c18c6e74

View File

@ -104,7 +104,7 @@ public final class NioEventLoop extends SingleThreadEventLoop {
private volatile int ioRatio = 50; private volatile int ioRatio = 50;
private int cancelledKeys; private int cancelledKeys;
private boolean cleanedCancelledKeys; private boolean needsToSelectAgain;
NioEventLoop( NioEventLoop(
NioEventLoopGroup parent, ThreadFactory threadFactory, NioEventLoopGroup parent, ThreadFactory threadFactory,
@ -399,12 +399,21 @@ public final class NioEventLoop extends SingleThreadEventLoop {
cancelledKeys ++; cancelledKeys ++;
if (cancelledKeys >= CLEANUP_INTERVAL) { if (cancelledKeys >= CLEANUP_INTERVAL) {
cancelledKeys = 0; cancelledKeys = 0;
cleanedCancelledKeys = true; needsToSelectAgain = true;
cleanupKeys();
} }
} }
@Override
protected Runnable pollTask() {
Runnable task = super.pollTask();
if (needsToSelectAgain) {
selectAgain();
}
return task;
}
private void processSelectedKeys() { private void processSelectedKeys() {
needsToSelectAgain = false;
Set<SelectionKey> selectedKeys = selector.selectedKeys(); Set<SelectionKey> selectedKeys = selector.selectedKeys();
// check if the set is empty and if so just return to not create garbage by // check if the set is empty and if so just return to not create garbage by
// creating a new Iterator every time even if there is nothing to process. // creating a new Iterator every time even if there is nothing to process.
@ -413,13 +422,12 @@ public final class NioEventLoop extends SingleThreadEventLoop {
return; return;
} }
Iterator<SelectionKey> i; Iterator<SelectionKey> i = selectedKeys.iterator();
cleanedCancelledKeys = false; for (;;) {
boolean clearSelectedKeys = true;
try {
for (i = selectedKeys.iterator(); i.hasNext();) {
final SelectionKey k = i.next(); final SelectionKey k = i.next();
final Object a = k.attachment(); final Object a = k.attachment();
i.remove();
if (a instanceof AbstractNioChannel) { if (a instanceof AbstractNioChannel) {
processSelectedKey(k, (AbstractNioChannel) a); processSelectedKey(k, (AbstractNioChannel) a);
} else { } else {
@ -428,21 +436,22 @@ public final class NioEventLoop extends SingleThreadEventLoop {
processSelectedKey(k, task); processSelectedKey(k, task);
} }
if (cleanedCancelledKeys) { if (!i.hasNext()) {
break;
}
if (needsToSelectAgain) {
selectAgain();
selectedKeys = selector.selectedKeys();
// Create the iterator again to avoid ConcurrentModificationException // Create the iterator again to avoid ConcurrentModificationException
if (selectedKeys.isEmpty()) { if (selectedKeys.isEmpty()) {
clearSelectedKeys = false;
break; break;
} else { } else {
i = selectedKeys.iterator(); i = selectedKeys.iterator();
} }
} }
} }
} finally {
if (clearSelectedKeys) {
selectedKeys.clear();
}
}
} }
private static void processSelectedKey(SelectionKey k, AbstractNioChannel ch) { private static void processSelectedKey(SelectionKey k, AbstractNioChannel ch) {
@ -534,7 +543,7 @@ public final class NioEventLoop extends SingleThreadEventLoop {
} }
private void closeAll() { private void closeAll() {
cleanupKeys(); selectAgain();
Set<SelectionKey> keys = selector.keys(); Set<SelectionKey> keys = selector.keys();
Collection<AbstractNioChannel> channels = new ArrayList<AbstractNioChannel>(keys.size()); Collection<AbstractNioChannel> channels = new ArrayList<AbstractNioChannel>(keys.size());
for (SelectionKey k: keys) { for (SelectionKey k: keys) {
@ -596,7 +605,8 @@ public final class NioEventLoop extends SingleThreadEventLoop {
return -1; return -1;
} }
void cleanupKeys() { private void selectAgain() {
needsToSelectAgain = false;
try { try {
selector.selectNow(); selector.selectNow();
} catch (Throwable t) { } catch (Throwable t) {