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>

common/src/main/java/io/netty/util/Recycler.java

@@ -396,12 +396,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 (!handle.hasBeenRecycled) {
                 if (handleRecycleCount < interval) {
                     handleRecycleCount++;
-                // Drop the item to prevent recycling to aggressive.
+                    // Drop the item to prevent from recycling too aggressively.
                     return;
                 }
                 handleRecycleCount = 0;
+            }
 
             Link tail = this.tail;
             int writeIndex;

common/src/test/java/io/netty/util/RecyclerTest.java

@@ -20,6 +20,8 @@ import org.junit.jupiter.api.Timeout;
 
 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;
@@ -328,6 +330,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