From 98a3a0c0cb7e2b9076da8d335549ae60970cf8fe Mon Sep 17 00:00:00 2001
From: ping <pj_zhong@163.com>
Date: Wed, 23 Jun 2021 17:32:44 +0800
Subject: [PATCH] Recycler.WeakOrderQueue drop Object hasBeenRecycled (#11402)

Motivation:

WeakOrderQueue would drop object that has been recycled, even when it has space for it.
WeakOrderQueue#add should check DefaultHandler.hasBeenRecycler field  first

Modifications:

WeakOrderQueue test the DefaultHandler.hasBeenRecycler first

Result:

WeakOrderQueue would not drop object that has been recycled when there is space


Co-authored-by: Norman Maurer <norman_maurer@apple.com>
Co-authored-by: Trustin Lee <t@motd.kr>
---
 .../src/main/java/io/netty/util/Recycler.java | 12 +++---
 .../test/java/io/netty/util/RecyclerTest.java | 39 +++++++++++++++++++
 2 files changed, 46 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 d64889488e..eec543f2d2 100644
--- a/common/src/main/java/io/netty/util/Recycler.java
+++ b/common/src/main/java/io/netty/util/Recycler.java
@@ -399,12 +399,14 @@ public abstract class Recycler<T> {
             // While we also enforce the recycling ratio when we transfer objects from the WeakOrderQueue to the Stack
             // we better should enforce it as well early. Missing to do so may let the WeakOrderQueue grow very fast
             // without control
-            if (handleRecycleCount < interval) {
-                handleRecycleCount++;
-                // Drop the item to prevent recycling to aggressive.
-                return;
+            if (!handle.hasBeenRecycled) {
+                if (handleRecycleCount < interval) {
+                    handleRecycleCount++;
+                    // Drop the item to prevent from recycling too aggressively.
+                    return;
+                }
+                handleRecycleCount = 0;
             }
-            handleRecycleCount = 0;
 
             Link tail = this.tail;
             int writeIndex;
diff --git a/common/src/test/java/io/netty/util/RecyclerTest.java b/common/src/test/java/io/netty/util/RecyclerTest.java
index a06523fcec..5d50e5981d 100644
--- a/common/src/test/java/io/netty/util/RecyclerTest.java
+++ b/common/src/test/java/io/netty/util/RecyclerTest.java
@@ -21,6 +21,8 @@ import org.junit.jupiter.api.function.Executable;
 
 import java.util.Random;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -345,6 +347,43 @@ public class RecyclerTest {
         assertNotSame(recycler.get(), o2);
     }
 
+    @Test
+    public void testRecycleAtTwoThreadsMulti() throws Exception {
+        final Recycler<HandledObject> recycler = newRecycler(256);
+        final HandledObject o = recycler.get();
+
+        ExecutorService single = Executors.newSingleThreadExecutor();
+
+        final CountDownLatch latch1 = new CountDownLatch(1);
+        single.execute(new Runnable() {
+            @Override
+            public void run() {
+                o.recycle();
+                latch1.countDown();
+            }
+        });
+        assertTrue(latch1.await(100, TimeUnit.MILLISECONDS));
+        final HandledObject o2 = recycler.get();
+        // Always recycler the first object, that is Ok
+        assertSame(o2, o);
+
+        final CountDownLatch latch2 = new CountDownLatch(1);
+        single.execute(new Runnable() {
+            @Override
+            public void run() {
+                //The object should be recycled
+                o2.recycle();
+                latch2.countDown();
+            }
+        });
+        assertTrue(latch2.await(100, TimeUnit.MILLISECONDS));
+
+        // It should be the same object, right?
+        final HandledObject o3 = recycler.get();
+        assertSame(o3, o);
+        single.shutdown();
+    }
+
     @Test
     public void testMaxCapacityWithRecycleAtDifferentThread() throws Exception {
         final int maxCapacity = 4; // Choose the number smaller than WeakOrderQueue.LINK_CAPACITY