Repository: netty/netty
Issue: Consider adding findNextPositivePowerOfTwo which does bound checks
Labels: defect

See:
https://github.com/netty/netty/pull/5594#discussion_r72509427


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java b/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java
index fb044c851a2..0caa6f1efc7 100644
--- a/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java
+++ b/buffer/src/main/java/io/netty/buffer/PoolThreadCache.java
@@ -369,7 +369,7 @@ private abstract static class MemoryRegionCache<T> {
         private int allocations;
 
         MemoryRegionCache(int size, SizeClass sizeClass) {
-            this.size = MathUtil.findNextPositivePowerOfTwo(size);
+            this.size = MathUtil.safeFindNextPositivePowerOfTwo(size);
             queue = PlatformDependent.newFixedMpscQueue(this.size);
             this.sizeClass = sizeClass;
         }
diff --git a/common/src/main/java/io/netty/util/Recycler.java b/common/src/main/java/io/netty/util/Recycler.java
index 71fd442e342..593974f5ecb 100644
--- a/common/src/main/java/io/netty/util/Recycler.java
+++ b/common/src/main/java/io/netty/util/Recycler.java
@@ -27,7 +27,7 @@
 import java.util.WeakHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
-import static io.netty.util.internal.MathUtil.findNextPositivePowerOfTwo;
+import static io.netty.util.internal.MathUtil.safeFindNextPositivePowerOfTwo;
 import static java.lang.Math.max;
 import static java.lang.Math.min;
 
@@ -71,14 +71,13 @@ public void recycle(Object object) {
                 SystemPropertyUtil.getInt("io.netty.recycler.maxSharedCapacityFactor",
                         2));
 
-        LINK_CAPACITY = findNextPositivePowerOfTwo(
+        LINK_CAPACITY = safeFindNextPositivePowerOfTwo(
                 max(SystemPropertyUtil.getInt("io.netty.recycler.linkCapacity", 16), 16));
 
         // By default we allow one push to a Recycler for each 8th try on handles that were never recycled before.
         // This should help to slowly increase the capacity of the recycler while not be too sensitive to allocation
         // bursts.
-        RATIO = min(findNextPositivePowerOfTwo(
-                max(SystemPropertyUtil.getInt("io.netty.recycler.ratio", 8), 2)), 0x40000000);
+        RATIO = safeFindNextPositivePowerOfTwo(SystemPropertyUtil.getInt("io.netty.recycler.ratio", 8));
 
         if (logger.isDebugEnabled()) {
             if (DEFAULT_MAX_CAPACITY == 0) {
@@ -121,10 +120,7 @@ protected Recycler(int maxCapacity, int maxSharedCapacityFactor) {
     }
 
     protected Recycler(int maxCapacity, int maxSharedCapacityFactor, int ratio) {
-        if (ratio > 0x40000000) {
-            throw new IllegalArgumentException(ratio + ": " + ratio + " (expected: < 0x40000000)");
-        }
-        ratioMask = findNextPositivePowerOfTwo(ratio) - 1;
+        ratioMask = safeFindNextPositivePowerOfTwo(ratio) - 1;
         if (maxCapacity <= 0) {
             this.maxCapacity = 0;
             this.maxSharedCapacityFactor = 1;
diff --git a/common/src/main/java/io/netty/util/ResourceLeakDetector.java b/common/src/main/java/io/netty/util/ResourceLeakDetector.java
index c0144e4b933..f85513aaf66 100644
--- a/common/src/main/java/io/netty/util/ResourceLeakDetector.java
+++ b/common/src/main/java/io/netty/util/ResourceLeakDetector.java
@@ -193,15 +193,12 @@ public ResourceLeakDetector(String resourceType, int samplingInterval, long maxA
         if (resourceType == null) {
             throw new NullPointerException("resourceType");
         }
-        if (samplingInterval <= 0) {
-            throw new IllegalArgumentException("samplingInterval: " + samplingInterval + " (expected: 1+)");
-        }
         if (maxActive <= 0) {
             throw new IllegalArgumentException("maxActive: " + maxActive + " (expected: 1+)");
         }
 
         this.resourceType = resourceType;
-        this.samplingInterval = MathUtil.findNextPositivePowerOfTwo(samplingInterval);
+        this.samplingInterval = MathUtil.safeFindNextPositivePowerOfTwo(samplingInterval);
         // samplingInterval is a power of two so we calculate a mask that we can use to
         // check if we need to do any leak detection or not.
         mask = this.samplingInterval - 1;
diff --git a/common/src/main/java/io/netty/util/internal/MathUtil.java b/common/src/main/java/io/netty/util/internal/MathUtil.java
index 2b16e22d7ce..10e012cf3f1 100644
--- a/common/src/main/java/io/netty/util/internal/MathUtil.java
+++ b/common/src/main/java/io/netty/util/internal/MathUtil.java
@@ -25,7 +25,7 @@ private MathUtil() {
     /**
      * Fast method of finding the next power of 2 greater than or equal to the supplied value.
      *
-     * If the value is {@code <= 0} then 1 will be returned.
+     * <p>If the value is {@code <= 0} then 1 will be returned.
      * This method is not suitable for {@link Integer#MIN_VALUE} or numbers greater than 2^30.
      *
      * @param value from which to search for next power of 2
@@ -36,6 +36,22 @@ public static int findNextPositivePowerOfTwo(final int value) {
         return 1 << (32 - Integer.numberOfLeadingZeros(value - 1));
     }
 
+    /**
+     * Fast method of finding the next power of 2 greater than or equal to the supplied value.
+     * <p>This method will do runtime bounds checking and call {@link #findNextPositivePowerOfTwo(int)} if within a
+     * valid range.
+     * @param value from which to search for next power of 2
+     * @return The next power of 2 or the value itself if it is a power of 2.
+     * <p>Special cases for return values are as follows:
+     * <ul>
+     *     <li>{@code <= 0} -> 1</li>
+     *     <li>{@code >= 2^30} -> 2^30</li>
+     * </ul>
+     */
+    public static int safeFindNextPositivePowerOfTwo(final int value) {
+        return value <= 0 ? 1 : value >= 0x40000000 ? 0x40000000 : findNextPositivePowerOfTwo(value);
+    }
+
     /**
      * Determine if the requested {@code index} and {@code length} will fit within {@code capacity}.
      * @param index The starting index.
diff --git a/common/src/main/templates/io/netty/util/collection/KObjectHashMap.template b/common/src/main/templates/io/netty/util/collection/KObjectHashMap.template
index 6358364c1eb..422e76e7f21 100644
--- a/common/src/main/templates/io/netty/util/collection/KObjectHashMap.template
+++ b/common/src/main/templates/io/netty/util/collection/KObjectHashMap.template
@@ -15,7 +15,7 @@
 
 package io.netty.util.collection;
 
-import static io.netty.util.internal.MathUtil.findNextPositivePowerOfTwo;
+import static io.netty.util.internal.MathUtil.safeFindNextPositivePowerOfTwo;
 
 import java.util.AbstractCollection;
 import java.util.AbstractSet;
@@ -77,9 +77,6 @@ public class @K@ObjectHashMap<V> implements @K@ObjectMap<V> {
     }
 
     public @K@ObjectHashMap(int initialCapacity, float loadFactor) {
-        if (initialCapacity < 1) {
-            throw new IllegalArgumentException("initialCapacity must be >= 1");
-        }
         if (loadFactor <= 0.0f || loadFactor > 1.0f) {
             // Cannot exceed 1 because we can never store more than capacity elements;
             // using a bigger loadFactor would trigger rehashing before the desired load is reached.
@@ -89,7 +86,7 @@ public class @K@ObjectHashMap<V> implements @K@ObjectMap<V> {
         this.loadFactor = loadFactor;
 
         // Adjust the initial capacity if necessary.
-        int capacity = findNextPositivePowerOfTwo(initialCapacity);
+        int capacity = safeFindNextPositivePowerOfTwo(initialCapacity);
         mask = capacity - 1;
 
         // Allocate the arrays.


Test patch: