Repository: netty/netty
Issue: Recycler will produce npe error when multiple recycled at different thread
Labels: defect

Netty version: 4.1.29.Final-SNAPSHOT

    public void testMultipleRecycleAtDifferentThread() {
        Recycler<HandledObject> recycler = newRecycler(1024);
        final HandledObject object = recycler.get();
        final Thread thread1 = new Thread(new Runnable() {
            @Override
            public void run() {
                object.recycle();  // stack=null
            }
        });
        thread1.start();
        try {
            thread1.join();
        } catch (InterruptedException e) {
        }

        final Thread thread2 = new Thread(new Runnable() {
            @Override
            public void run() {
                object.recycle();  //  stack npe 
            }
        });
        thread2.start();
        try {
            thread2.join();
        } catch (InterruptedException e) {
        }
    }


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/common/src/main/java/io/netty/util/Recycler.java b/common/src/main/java/io/netty/util/Recycler.java
index 491464d9a31..5e7db4235b1 100644
--- a/common/src/main/java/io/netty/util/Recycler.java
+++ b/common/src/main/java/io/netty/util/Recycler.java
@@ -216,6 +216,11 @@ public void recycle(Object object) {
             if (object != value) {
                 throw new IllegalArgumentException("object does not belong to handle");
             }
+
+            if (this.lastRecycledId != this.recycleId) {
+                throw new IllegalStateException("recycled already");
+            }
+
             stack.push(this);
         }
     }


Test patch:
diff --git a/common/src/test/java/io/netty/util/RecyclerTest.java b/common/src/test/java/io/netty/util/RecyclerTest.java
index 6eeace5f001..e4bcbf1da46 100644
--- a/common/src/test/java/io/netty/util/RecyclerTest.java
+++ b/common/src/test/java/io/netty/util/RecyclerTest.java
@@ -81,6 +81,38 @@ public void testMultipleRecycle() {
         object.recycle();
     }
 
+    @Test(expected = IllegalStateException.class)
+    public void testMultipleRecycleAtDifferentThread() throws InterruptedException {
+        Recycler<HandledObject> recycler = newRecycler(1024);
+        final HandledObject object = recycler.get();
+        final AtomicReference<IllegalStateException> exceptionStore = new AtomicReference<IllegalStateException>();
+        final Thread thread1 = new Thread(new Runnable() {
+            @Override
+            public void run() {
+                object.recycle();
+            }
+        });
+        thread1.start();
+        thread1.join();
+
+        final Thread thread2 = new Thread(new Runnable() {
+            @Override
+            public void run() {
+                try {
+                    object.recycle();
+                } catch (IllegalStateException e) {
+                    exceptionStore.set(e);
+                }
+            }
+        });
+        thread2.start();
+        thread2.join();
+        IllegalStateException exception = exceptionStore.get();
+        if (exception != null) {
+            throw exception;
+        }
+    }
+
     @Test
     public void testRecycle() {
         Recycler<HandledObject> recycler = newRecycler(1024);