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:
parent
9b99a611ad
commit
98a533ae44
@ -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;
|
||||
|
@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user