From 341e4f11418dc85c93ce64852ce80bbc83e7d1af Mon Sep 17 00:00:00 2001
From: Chris Vest <christianvest_hansen@apple.com>
Date: Wed, 7 Sep 2022 06:13:32 -0700
Subject: [PATCH] Fix inconsistent synchronization of PoolSubpage.doNotDestroy
 (#12775)

Motivation:
This field is modified under lock, and thus should not be accessed (toString excepted) without locking.

Modification:
Move an assert check on PoolSubpage.doNotDestroy into a nearby critical region.

Also encapsulate the findSubpagePoolHead logic better; PoolArena now always use this method instead of that one place that was accessing the array directly.
This should not have any influence on the logic.

Result:
Assert in PoolChunk is no longer racy.
---
 buffer/src/main/java/io/netty/buffer/PoolArena.java | 2 +-
 buffer/src/main/java/io/netty/buffer/PoolChunk.java | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/buffer/src/main/java/io/netty/buffer/PoolArena.java b/buffer/src/main/java/io/netty/buffer/PoolArena.java
index e4a3e27b182..8ab313466cd 100644
--- a/buffer/src/main/java/io/netty/buffer/PoolArena.java
+++ b/buffer/src/main/java/io/netty/buffer/PoolArena.java
@@ -157,7 +157,7 @@ private void tcacheAllocateSmall(PoolThreadCache cache, PooledByteBuf<T> buf, fi
          * Synchronize on the head. This is needed as {@link PoolChunk#allocateSubpage(int)} and
          * {@link PoolChunk#free(long)} may modify the doubly linked list as well.
          */
-        final PoolSubpage<T> head = smallSubpagePools[sizeIdx];
+        final PoolSubpage<T> head = findSubpagePoolHead(sizeIdx);
         final boolean needsNormalAllocation;
         head.lock();
         try {
diff --git a/buffer/src/main/java/io/netty/buffer/PoolChunk.java b/buffer/src/main/java/io/netty/buffer/PoolChunk.java
index c7a0ffc22b1..bb1efff2a3a 100644
--- a/buffer/src/main/java/io/netty/buffer/PoolChunk.java
+++ b/buffer/src/main/java/io/netty/buffer/PoolChunk.java
@@ -479,12 +479,12 @@ void free(long handle, int normCapacity, ByteBuffer nioBuffer) {
 
             int sIdx = runOffset(handle);
             PoolSubpage<T> subpage = subpages[sIdx];
-            assert subpage != null && subpage.doNotDestroy;
 
             // Obtain the head of the PoolSubPage pool that is owned by the PoolArena and synchronize on it.
             // This is need as we may add it back and so alter the linked-list structure.
             head.lock();
             try {
+                assert subpage != null && subpage.doNotDestroy;
                 if (subpage.free(head, bitmapIdx(handle))) {
                     //the subpage is still used, do not free it
                     return;