fix review

fix review comment
add new ut

Author: walterzhaoJR

src/butil/iobuf.cpp

@@ -324,33 +324,32 @@ IOBuf::Block* share_tls_block() {
 void release_tls_block_chain(IOBuf::Block* b) {
     TLSData& tls_data = g_tls_data;
     size_t n = 0;
+    size_t n_hit_threshold = 0;
     if (tls_data.num_blocks >= max_blocks_per_thread()) {
         do {
             ++n;
             IOBuf::Block* const saved_next = b->u.portal_next;
-            b->dec_ref();
+            // If b is already cached in TLS, dec_ref() would drop a reference
+            // still owned by the TLS list and leave a dangling pointer behind.
+            if (!is_in_tls_block_chain(tls_data.block_head, b)) {
+                b->dec_ref();
+                ++n_hit_threshold;
+            }
             b = saved_next;
         } while (b);
-        g_num_hit_tls_threshold.fetch_add(n, butil::memory_order_relaxed);
+        g_num_hit_tls_threshold.fetch_add(
+            n_hit_threshold, butil::memory_order_relaxed);
         return;
     }
-    IOBuf::Block* const old_head = tls_data.block_head;
     IOBuf::Block* first_b = b;
     IOBuf::Block* last_b = NULL;
     do {
         ++n;
         CHECK(!b->full());
-        // If any block in the incoming chain is already the TLS block_head,
-        // linking last_b->portal_next = block_head would create a cycle:
-        //   - Single-block chain [B] where B == block_head:
-        //     last_b->portal_next = B, i.e. B->portal_next = B (self-loop)
-        //   - Multi-block chain [A -> B] where A == block_head:
-        //     last_b(B)->portal_next = A, creating A -> B -> A (cycle)
-        // Return early before any state is modified so that num_blocks stays
-        // consistent with the actual list length (remove_tls_block_chain
-        // verifies this with CHECK_EQ at thread exit).
-        // See https://github.com/apache/brpc/issues/3243
-        if (b == old_head) {
+        // Guard against overlap with any node already cached in TLS, not just
+        // block_head.  Otherwise returning X into H -> X would create a
+        // 2-node cycle X -> H -> X and hang later list traversal.
+        if (is_in_tls_block_chain(tls_data.block_head, b)) {
             return;
         }
         if (b->u.portal_next == NULL) {
@@ -359,7 +358,7 @@ void release_tls_block_chain(IOBuf::Block* b) {
         }
         b = b->u.portal_next;
     } while (true);
-    last_b->u.portal_next = old_head;
+    last_b->u.portal_next = tls_data.block_head;
     tls_data.block_head = first_b;
     tls_data.num_blocks += n;
     if (!tls_data.registered) {

src/butil/iobuf_inl.h

@@ -603,26 +603,34 @@ void remove_tls_block_chain();
 
 IOBuf::Block* acquire_tls_block();
 
+inline bool is_in_tls_block_chain(IOBuf::Block* head, IOBuf::Block* b) {
+    for (IOBuf::Block* p = head; p != NULL; p = p->u.portal_next) {
+        if (p == b) {
+            return true;
+        }
+    }
+    return false;
+}
+
 // Return one block to TLS.
 inline void release_tls_block(IOBuf::Block* b) {
     if (!b) {
         return;
     }
     TLSData *tls_data = get_g_tls_data();
+    // Guard against duplicate return anywhere in the TLS list.  Checking only
+    // block_head misses cases like H -> X where returning X again would create
+    // a 2-node cycle X -> H -> X.  The TLS list is short (soft-limited), so a
+    // linear scan is cheap here.
+    if (is_in_tls_block_chain(tls_data->block_head, b)) {
+        return;
+    }
     if (b->full()) {
         b->dec_ref();
     } else if (tls_data->num_blocks >= max_blocks_per_thread()) {
         b->dec_ref();
         // g_num_hit_tls_threshold.fetch_add(1, butil::memory_order_relaxed);
         inc_g_num_hit_tls_threshold();
-    } else if (b == tls_data->block_head) {
-        // b is already at the head of the TLS free list.  Re-inserting it
-        // would execute `b->portal_next = block_head`, i.e. `b->portal_next = b`,
-        // creating a single-node self-loop.  Any later traversal of the TLS
-        // chain (remove_tls_block_chain at thread exit, share_tls_block, etc.)
-        // would spin forever.  Skip the duplicate return.
-        // See https://github.com/apache/brpc/issues/3243
-        return;
     } else {
         b->u.portal_next = tls_data->block_head;
         tls_data->block_head = b;