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.
This commit is contained in:
Amir Szekely 2014-08-30 17:39:03 -07:00 committed by Norman Maurer
parent cc55a71b14
commit bdd950dff8
2 changed files with 54 additions and 5 deletions

View File

@ -96,6 +96,10 @@ public abstract class Recycler<T> {
return true; return true;
} }
final int threadLocalCapacity() {
return threadLocal.get().elements.length;
}
protected abstract T newObject(Handle<T> handle); protected abstract T newObject(Handle<T> handle);
public interface Handle<T> { public interface Handle<T> {
@ -339,12 +343,12 @@ public abstract class Recycler<T> {
item.recycleId = item.lastRecycledId = OWN_THREAD_ID; item.recycleId = item.lastRecycledId = OWN_THREAD_ID;
int size = this.size; int size = this.size;
if (size == elements.length) {
if (size == maxCapacity) { if (size == maxCapacity) {
// Hit the maximum capacity - drop the possibly youngest object. // Hit the maximum capacity - drop the possibly youngest object.
return; return;
} }
elements = Arrays.copyOf(elements, size << 1); if (size == elements.length) {
elements = Arrays.copyOf(elements, Math.min(size << 1, maxCapacity));
} }
elements[size] = item; elements[size] = item;

View File

@ -15,6 +15,8 @@
*/ */
package io.netty.util; package io.netty.util;
import java.util.Random;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Test; import org.junit.Test;
@ -59,4 +61,47 @@ public class RecyclerTest {
RECYCLER.recycle(this, handle); 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;
}
}
} }