Fix IOBuf TLS block pool self-loop caused by double-return (#3243) by walterzhaoJR · Pull Request #3244 · apache/brpc

walterzhaoJR · 2026-03-15T16:49:49Z

release_tls_block() and release_tls_block_chain() do not guard against a block being returned to TLS when it is already the list head. The assignment b->portal_next = block_head becomes b->portal_next = b, forming a self-loop that causes remove_tls_block_chain() or share_tls_block() to spin forever, silently hanging the thread at exit.

Fix:

release_tls_block(): add early return when b == block_head, skipping the duplicate insertion.
release_tls_block_chain(): during the existing chain walk, check each node against block_head before linking. Return early if overlap is detected so that num_blocks stays consistent with the actual list length (remove_tls_block_chain verifies this via CHECK_EQ).

Add three unit tests that reproduce the self-loop through:

Direct double release_tls_block() of the same block.
release_tls_block_chain() with a chain overlapping the TLS head.
IOBufAsZeroCopyOutputStream::BackUp() followed by a second release.

What problem does this PR solve?

Issue Number: resolve

Problem Summary:

What is changed and the side effects?

Changed:

Side effects:

Performance effects:
Breaking backward compatibility:

Check List:

Please make sure your changes are compilable.
When providing us with a new feature, it is best to add related tests.
Please follow Contributor Covenant Code of Conduct.

release_tls_block() and release_tls_block_chain() do not guard against a block being returned to TLS when it is already the list head. The assignment `b->portal_next = block_head` becomes `b->portal_next = b`, forming a self-loop that causes remove_tls_block_chain() or share_tls_block() to spin forever, silently hanging the thread at exit. Fix: - release_tls_block(): add early return when b == block_head, skipping the duplicate insertion. - release_tls_block_chain(): during the existing chain walk, check each node against block_head before linking. Return early if overlap is detected so that num_blocks stays consistent with the actual list length (remove_tls_block_chain verifies this via CHECK_EQ). Add three unit tests that reproduce the self-loop through: 1. Direct double release_tls_block() of the same block. 2. release_tls_block_chain() with a chain overlapping the TLS head. 3. IOBufAsZeroCopyOutputStream::BackUp() followed by a second release.

Copilot

Pull request overview

Fixes a TLS IOBuf block-pool corruption case where double-returning a block can create a cyclic free-list and hang the thread on later traversal (notably at thread exit cleanup).

Changes:

Add a guard in release_tls_block() intended to prevent re-inserting the TLS head and forming a self-loop.
Add an overlap check in release_tls_block_chain() intended to prevent linking a chain that would create a cycle and to keep num_blocks consistent.
Add three unit tests reproducing the reported hang scenarios from issue #3243.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File	Description
`src/butil/iobuf_inl.h`	Adds a duplicate-return guard in `release_tls_block()` to prevent self-looping the TLS free list.
`src/butil/iobuf.cpp`	Adds an overlap check and uses a saved `old_head` when returning a block chain to TLS.
`test/iobuf_unittest.cpp`	Adds regression tests that attempt to reproduce and detect the TLS free-list cycle described in #3243.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/butil/iobuf.cpp

@@ -334,18 +334,32 @@ void release_tls_block_chain(IOBuf::Block* b) {
        g_num_hit_tls_threshold.fetch_add(n, butil::memory_order_relaxed);
        return;
    }
+    IOBuf::Block* const old_head = tls_data.block_head;


test/iobuf_unittest.cpp

+        // acquire_tls_block() sets portal_next = NULL, breaking the loop.
+        butil::iobuf::acquire_tls_block();
+        // block_head is still b (was self-referencing), acquire again to drain.
+        butil::iobuf::acquire_tls_block();
+        // TLS is now empty — thread can exit without hanging.


src/butil/iobuf_inl.h

@@ -615,6 +615,14 @@ inline void release_tls_block(IOBuf::Block* b) {
        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) {


src/butil/iobuf.cpp

+        // 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:


test/iobuf_unittest.cpp

+        butil::iobuf::acquire_tls_block();
+        butil::iobuf::acquire_tls_block();


test/iobuf_unittest.cpp

+            butil::iobuf::acquire_tls_block();
+            butil::iobuf::acquire_tls_block();


test/iobuf_unittest.cpp

+TEST_F(IOBufTest, release_tls_block_double_return_creates_self_loop) {
+    bool self_loop = false;
+    pthread_t tid;
+    ASSERT_EQ(0, pthread_create(&tid, NULL,
+                                double_return_release_tls_block_thread,
+                                &self_loop));
+    ASSERT_EQ(0, pthread_join(tid, NULL));


test/iobuf_unittest.cpp

+TEST_F(IOBufTest, release_tls_block_chain_overlap_creates_self_loop) {
+    bool self_loop = false;
+    pthread_t tid;
+    ASSERT_EQ(0, pthread_create(&tid, NULL,
+                                double_return_release_tls_block_chain_thread,
+                                &self_loop));
+    ASSERT_EQ(0, pthread_join(tid, NULL));


test/iobuf_unittest.cpp

+TEST_F(IOBufTest, backup_then_double_release_creates_self_loop) {
+    bool self_loop = false;
+    pthread_t tid;
+    ASSERT_EQ(0, pthread_create(&tid, NULL,
+                                backup_double_return_thread, &self_loop));
+    ASSERT_EQ(0, pthread_join(tid, NULL));
+
+    EXPECT_FALSE(self_loop)
+        << "After IOBufAsZeroCopyOutputStream::BackUp() returned a block to "


fix review comment add new ut

add more ut

yanglimingcn requested a review from Copilot March 16, 2026 02:06

Copilot started reviewing on behalf of yanglimingcn March 16, 2026 02:07 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

chenBright linked an issue Mar 16, 2026 that may be closed by this pull request

[Bug] IOBuf TLS block pool: double-return of a Block creates a self-loop in portal_next linked list, causing thread hang #3243

Open

walterzhaoJR added 2 commits March 16, 2026 11:34

fix review

328c6b7

fix review comment add new ut

fix review comment#2

b615fa4

add more ut

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix IOBuf TLS block pool self-loop caused by double-return (#3243)#3244

Fix IOBuf TLS block pool self-loop caused by double-return (#3243)#3244
walterzhaoJR wants to merge 3 commits intoapache:masterfrom
walterzhaoJR:fix-3243

walterzhaoJR commented Mar 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		butil::iobuf::acquire_tls_block();
		butil::iobuf::acquire_tls_block();

Conversation

walterzhaoJR commented Mar 15, 2026

What problem does this PR solve?

What is changed and the side effects?

Check List:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants