andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix overflow bug in GenericEventExecutorChooser (#10468)

Browse Source 
Motivation:
 The executor chooser should pluck executors in round-robin, but at the 32-bit
 overflow boundary, the round-robin sequence was disrupted when the number of
 executors are not a power of 2.

Modification:
 Changed the index counter from a 32-bit to a 64-bit long. The overflow bug is
 still technically there, but it now takes so long to reach that it will never
 happen in practice. For example, 2^63 nanoseconds is almost 300 years.

Result:
 The round-robin behaviour for all EventExecutorChoosers is now preserved in
 practice.
 This fixes #10423.
This commit is contained in:
Chris Vest 2020-08-11 20:53:48 +02:00 committed by GitHub
parent 3d7ec896ac
commit cfeda0fef2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 6 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
common/src/main/java/io/netty/util/concurrent/DefaultEventExecutorChooserFactory.java
View File

@ -18,6 +18,7 @@ package io.netty.util.concurrent;
import io.netty.util.internal.UnstableApi;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
/**
* Default implementation which uses simple round-robin to choose next {@link EventExecutor}.
@ -29,7 +30,6 @@ public final class DefaultEventExecutorChooserFactory implements EventExecutorCh
private DefaultEventExecutorChooserFactory() { }
@SuppressWarnings("unchecked")
@Override
public EventExecutorChooser newChooser(EventExecutor[] executors) {
if (isPowerOfTwo(executors.length)) {
@ -58,7 +58,10 @@ public final class DefaultEventExecutorChooserFactory implements EventExecutorCh
}
private static final class GenericEventExecutorChooser implements EventExecutorChooser {
private final AtomicInteger idx = new AtomicInteger();
// Use a 'long' counter to avoid non-round-robin behaviour at the 32-bit overflow boundary.
// The 64-bit long solves this by placing the overflow so far into the future, that no system
// will encounter this in practice.
private final AtomicLong idx = new AtomicLong();
private final EventExecutor[] executors;
GenericEventExecutorChooser(EventExecutor[] executors) {
@ -67,7 +70,7 @@ public final class DefaultEventExecutorChooserFactory implements EventExecutorCh
@Override
public EventExecutor next() {
return executors[Math.abs(idx.getAndIncrement() % executors.length)];
return executors[(int) Math.abs(idx.getAndIncrement() % executors.length)];
}
}
}