From cfeda0fef26ca52007ae664c967f24e3f25236f0 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Tue, 11 Aug 2020 20:53:48 +0200 Subject: [PATCH] Fix overflow bug in GenericEventExecutorChooser (#10468) 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. --- .../concurrent/DefaultEventExecutorChooserFactory.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/common/src/main/java/io/netty/util/concurrent/DefaultEventExecutorChooserFactory.java b/common/src/main/java/io/netty/util/concurrent/DefaultEventExecutorChooserFactory.java index 11bf8643aa..10070ccade 100644 --- a/common/src/main/java/io/netty/util/concurrent/DefaultEventExecutorChooserFactory.java +++ b/common/src/main/java/io/netty/util/concurrent/DefaultEventExecutorChooserFactory.java @@ -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)]; } } }