From bdd950dff84f36718334929e8e0332e559123a2a Mon Sep 17 00:00:00 2001
From: Amir Szekely <kichik@gmail.com>
Date: Sat, 30 Aug 2014 17:39:03 -0700
Subject: [PATCH] Don't ignore maxCapacity if it's not a power of 2

Motivation:

This fixes bug #2848 which caused Recycler to become unbounded and cache infinite number of objects with maxCapacity that's not a power of two. This can result in general sluggishness of the application and OutOfMemoryError.

Modifications:

The test for maxCapacity has been moved out of test to check if the buffer has filled. The buffer is now also capped at maxCapacity and cannot grow over it as it jumps from one power of two to the other.

Additionally, a unit test was added to verify maxCapacity is honored even when it's not a power of two.

Result:

With these changes the user is able to use a custom maxCapacity number and not have it ignored. The unit test assures this bug will not repeat itself.
---
 .../src/main/java/io/netty/util/Recycler.java | 14 +++---
 .../test/java/io/netty/util/RecyclerTest.java | 45 +++++++++++++++++++
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/common/src/main/java/io/netty/util/Recycler.java b/common/src/main/java/io/netty/util/Recycler.java
index a56499cd7b..0600f66d70 100644
--- a/common/src/main/java/io/netty/util/Recycler.java
+++ b/common/src/main/java/io/netty/util/Recycler.java
@@ -96,6 +96,10 @@ public abstract class Recycler<T> {
         return true;
     }
 
+    final int threadLocalCapacity() {
+        return threadLocal.get().elements.length;
+    }
+
     protected abstract T newObject(Handle<T> handle);
 
     public interface Handle<T> {
@@ -339,12 +343,12 @@ public abstract class Recycler<T> {
             item.recycleId = item.lastRecycledId = OWN_THREAD_ID;
 
             int size = this.size;
+            if (size == maxCapacity) {
+                // Hit the maximum capacity - drop the possibly youngest object.
+                return;
+            }
             if (size == elements.length) {
-                if (size == maxCapacity) {
-                    // Hit the maximum capacity - drop the possibly youngest object.
-                    return;
-                }
-                elements = Arrays.copyOf(elements, size << 1);
+                elements = Arrays.copyOf(elements, Math.min(size << 1, maxCapacity));
             }
 
             elements[size] = item;
diff --git a/common/src/test/java/io/netty/util/RecyclerTest.java b/common/src/test/java/io/netty/util/RecyclerTest.java
index 5212e9b4df..91114344b6 100644
--- a/common/src/test/java/io/netty/util/RecyclerTest.java
+++ b/common/src/test/java/io/netty/util/RecyclerTest.java
@@ -15,6 +15,8 @@
 */
 package io.netty.util;
 
+import java.util.Random;
+
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -59,4 +61,47 @@ public class RecyclerTest {
             RECYCLER.recycle(this, handle);
         }
     }
+
+    /**
+     * Test to make sure bug #2848 never happens again
+     * https://github.com/netty/netty/issues/2848
+     */
+    @Test
+    public void testMaxCapacity() {
+        testMaxCapacity(300);
+        Random rand = new Random();
+        for (int i = 0; i < 50; i++) {
+            testMaxCapacity(rand.nextInt(1000) + 256); // 256 - 1256
+        }
+    }
+
+    void testMaxCapacity(int maxCapacity) {
+        Recycler<HandledObject> recycler = new Recycler<HandledObject>(maxCapacity) {
+            @Override
+            protected HandledObject newObject(
+                    Recycler.Handle<HandledObject> handle) {
+                return new HandledObject(handle);
+            }
+        };
+
+        HandledObject[] objects = new HandledObject[maxCapacity * 3];
+        for (int i = 0; i < objects.length; i++) {
+            objects[i] = recycler.get();
+        }
+
+        for (int i = 0; i < objects.length; i++) {
+            recycler.recycle(objects[i], objects[i].handle);
+            objects[i] = null;
+        }
+
+        Assert.assertEquals(maxCapacity, recycler.threadLocalCapacity());
+    }
+
+    static final class HandledObject {
+        Recycler.Handle<HandledObject> handle;
+
+        HandledObject(Recycler.Handle<HandledObject> handle) {
+            this.handle = handle;
+        }
+    }
 }