Ensure leak aware buffers correctly close the ResourceLeakTracker

Motivation: We should assert that the leak aware buffers correctly close the ResourceLeakTracker in the unit tests. Modifications: - Keep track of NoopResourceLeakTrackers and check if these were closed once the test completes - Fix bugs in tests so the buffers are all released. Result: Better tests for leak aware buffers
netty · Dec 8, 2016 · 712c16a · 712c16a
1 parent f6ac8b5
commit 712c16a
Show file tree

Hide file tree

Showing 4 changed files with 116 additions and 20 deletions.
diff --git a/buffer/src/test/java/io/netty/buffer/AdvancedLeakAwareByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AdvancedLeakAwareByteBufTest.java
@@ -15,6 +15,8 @@
  */
 package io.netty.buffer;
 
+import io.netty.util.ResourceLeakTracker;
+
 public class AdvancedLeakAwareByteBufTest extends SimpleLeakAwareByteBufTest {
 
     @Override
@@ -23,7 +25,7 @@ protected Class<? extends ByteBuf> leakClass() {
     }
 
     @Override
-    protected ByteBuf wrap(ByteBuf buffer) {
-        return new AdvancedLeakAwareByteBuf(buffer, new NoopResourceLeakTracker<ByteBuf>());
+    protected SimpleLeakAwareByteBuf wrap(ByteBuf buffer, ResourceLeakTracker<ByteBuf> tracker) {
+        return new AdvancedLeakAwareByteBuf(buffer, tracker);
     }
 }
diff --git a/buffer/src/test/java/io/netty/buffer/AdvancedLeakAwareCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AdvancedLeakAwareCompositeByteBufTest.java
@@ -15,11 +15,13 @@
  */
 package io.netty.buffer;
 
+import io.netty.util.ResourceLeakTracker;
+
 public class AdvancedLeakAwareCompositeByteBufTest extends SimpleLeakAwareCompositeByteBufTest {
 
     @Override
-    protected WrappedCompositeByteBuf wrap(CompositeByteBuf buffer) {
-        return new AdvancedLeakAwareCompositeByteBuf(buffer, new NoopResourceLeakTracker<ByteBuf>());
+    protected SimpleLeakAwareCompositeByteBuf wrap(CompositeByteBuf buffer, ResourceLeakTracker<ByteBuf> tracker) {
+        return new AdvancedLeakAwareCompositeByteBuf(buffer, tracker);
     }
 
     @Override

diff --git a/buffer/src/test/java/io/netty/buffer/SimpleLeakAwareByteBufTest.java b/buffer/src/test/java/io/netty/buffer/SimpleLeakAwareByteBufTest.java
@@ -15,19 +15,57 @@
  */
 package io.netty.buffer;
 
-import org.junit.Assert;
+import io.netty.util.ResourceLeakTracker;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 
+import java.util.ArrayDeque;
+import java.util.Queue;
+
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
 public class SimpleLeakAwareByteBufTest extends BigEndianHeapByteBufTest {
     private final Class<? extends ByteBuf> clazz = leakClass();
+    private final Queue<NoopResourceLeakTracker<ByteBuf>> trackers = new ArrayDeque<NoopResourceLeakTracker<ByteBuf>>();
 
     @Override
     protected final ByteBuf newBuffer(int capacity) {
         return wrap(super.newBuffer(capacity));
     }
 
-    protected ByteBuf wrap(ByteBuf buffer) {
-        return new SimpleLeakAwareByteBuf(buffer, new NoopResourceLeakTracker<ByteBuf>());
+    private ByteBuf wrap(ByteBuf buffer) {
+        NoopResourceLeakTracker<ByteBuf> tracker = new NoopResourceLeakTracker<ByteBuf>();
+        ByteBuf leakAwareBuf = wrap(buffer, tracker);
+        trackers.add(tracker);
+        return leakAwareBuf;
+    }
+
+    protected SimpleLeakAwareByteBuf wrap(ByteBuf buffer, ResourceLeakTracker<ByteBuf> tracker) {
+        return new SimpleLeakAwareByteBuf(buffer, tracker);
+    }
+
+    @Before
+    @Override
+    public void init() {
+        super.init();
+        trackers.clear();
+    }
+
+    @After
+    @Override
+    public void dispose() {
+        super.dispose();
+
+        for (;;) {
+            NoopResourceLeakTracker<ByteBuf> tracker = trackers.poll();
+
+            if (tracker == null) {
+                break;
+            }
+            assertTrue(tracker.get());
+        }
     }
 
     protected Class<? extends ByteBuf> leakClass() {
@@ -51,17 +89,23 @@ public void testWrapReadSlice() {
 
     @Test
     public void testWrapRetainedSlice() {
-        assertWrapped(newBuffer(8).retainedSlice());
+        ByteBuf buffer = newBuffer(8);
+        assertWrapped(buffer.retainedSlice());
+        assertTrue(buffer.release());
     }
 
     @Test
     public void testWrapRetainedSlice2() {
-        assertWrapped(newBuffer(8).retainedSlice(0, 1));
+        ByteBuf buffer = newBuffer(8);
+        assertWrapped(buffer.retainedSlice(0, 1));
+        assertTrue(buffer.release());
     }
 
     @Test
     public void testWrapReadRetainedSlice() {
-        assertWrapped(newBuffer(8).readRetainedSlice(1));
+        ByteBuf buffer = newBuffer(8);
+        assertWrapped(buffer.readRetainedSlice(1));
+        assertTrue(buffer.release());
     }
 
     @Test
@@ -71,7 +115,9 @@ public void testWrapDuplicate() {
 
     @Test
     public void testWrapRetainedDuplicate() {
-        assertWrapped(newBuffer(8).retainedDuplicate());
+        ByteBuf buffer = newBuffer(8);
+        assertWrapped(buffer.retainedDuplicate());
+        assertTrue(buffer.release());
     }
 
     @Test
@@ -81,7 +127,7 @@ public void testWrapReadOnly() {
 
     protected final void assertWrapped(ByteBuf buf) {
         try {
-            Assert.assertSame(clazz, buf.getClass());
+            assertSame(clazz, buf.getClass());
         } finally {
             buf.release();
         }

diff --git a/buffer/src/test/java/io/netty/buffer/SimpleLeakAwareCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/SimpleLeakAwareCompositeByteBufTest.java
@@ -15,16 +15,54 @@
  */
 package io.netty.buffer;
 
-import org.junit.Assert;
+import io.netty.util.ResourceLeakTracker;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 
+import java.util.ArrayDeque;
+import java.util.Queue;
+
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertTrue;
+
 public class SimpleLeakAwareCompositeByteBufTest extends WrappedCompositeByteBufTest {
 
     private final Class<? extends ByteBuf> clazz = leakClass();
+    private final Queue<NoopResourceLeakTracker<ByteBuf>> trackers = new ArrayDeque<NoopResourceLeakTracker<ByteBuf>>();
+
+    @Override
+    protected final WrappedCompositeByteBuf wrap(CompositeByteBuf buffer) {
+        NoopResourceLeakTracker<ByteBuf> tracker = new NoopResourceLeakTracker<ByteBuf>();
+        WrappedCompositeByteBuf leakAwareBuf = wrap(buffer, tracker);
+        trackers.add(tracker);
+        return leakAwareBuf;
+    }
+
+    protected SimpleLeakAwareCompositeByteBuf wrap(CompositeByteBuf buffer, ResourceLeakTracker<ByteBuf> tracker) {
+        return new SimpleLeakAwareCompositeByteBuf(buffer, tracker);
+    }
 
+    @Before
     @Override
-    protected WrappedCompositeByteBuf wrap(CompositeByteBuf buffer) {
-        return new SimpleLeakAwareCompositeByteBuf(buffer, new NoopResourceLeakTracker<ByteBuf>());
+    public void init() {
+        super.init();
+        trackers.clear();
+    }
+
+    @After
+    @Override
+    public void dispose() {
+        super.dispose();
+
+        for (;;) {
+            NoopResourceLeakTracker<ByteBuf> tracker = trackers.poll();
+
+            if (tracker == null) {
+                break;
+            }
+            assertTrue(tracker.get());
+        }
     }
 
     protected Class<? extends ByteBuf> leakClass() {
@@ -48,17 +86,23 @@ public void testWrapReadSlice() {
 
     @Test
     public void testWrapRetainedSlice() {
-        assertWrapped(newBuffer(8).retainedSlice());
+        ByteBuf buffer = newBuffer(8);
+        assertWrapped(buffer.retainedSlice());
+        assertTrue(buffer.release());
     }
 
     @Test
     public void testWrapRetainedSlice2() {
-        assertWrapped(newBuffer(8).retainedSlice(0, 1));
+        ByteBuf buffer = newBuffer(8);
+        assertWrapped(buffer.retainedSlice(0, 1));
+        assertTrue(buffer.release());
     }
 
     @Test
     public void testWrapReadRetainedSlice() {
-        assertWrapped(newBuffer(8).readRetainedSlice(1));
+        ByteBuf buffer = newBuffer(8);
+        assertWrapped(buffer.readRetainedSlice(1));
+        assertTrue(buffer.release());
     }
 
     @Test
@@ -68,7 +112,9 @@ public void testWrapDuplicate() {
 
     @Test
     public void testWrapRetainedDuplicate() {
-        assertWrapped(newBuffer(8).retainedDuplicate());
+        ByteBuf buffer = newBuffer(8);
+        assertWrapped(buffer.retainedDuplicate());
+        assertTrue(buffer.release());
     }
 
     @Test
@@ -78,7 +124,7 @@ public void testWrapReadOnly() {
 
     protected final void assertWrapped(ByteBuf buf) {
         try {
-            Assert.assertSame(clazz, buf.getClass());
+            assertSame(clazz, buf.getClass());
         } finally {
             buf.release();
         }