From cf888ad87b70118c4e108e7f3ba4759a7525c815 Mon Sep 17 00:00:00 2001
From: songgaoye <217724508+songgaoye@users.noreply.github.com>
Date: Tue, 16 Jun 2026 17:22:38 +0800
Subject: [PATCH] fix(blocksync): fix deadlock in AddBlock caused by holding
 pool.mtx during sendError

---
 blocksync/pool.go      | 18 +++++++++++-------
 blocksync/pool_test.go | 40 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/blocksync/pool.go b/blocksync/pool.go
index 692e8e968a..7ca8896716 100644
--- a/blocksync/pool.go
+++ b/blocksync/pool.go
@@ -366,7 +366,13 @@ func (pool *BlockPool) AddBlock(peerID p2p.ID, block *types.Block, extCommit *ty
 	}
 
 	pool.mtx.Lock()
-	defer pool.mtx.Unlock()
+	var sendErr error
+	defer func() {
+		pool.mtx.Unlock()
+		if sendErr != nil {
+			pool.sendError(sendErr, peerID)
+		}
+	}()
 
 	requester := pool.requesters[block.Height]
 	if requester == nil {
@@ -375,19 +381,17 @@ func (pool *BlockPool) AddBlock(peerID p2p.ID, block *types.Block, extCommit *ty
 		// can't punish it. But if the peer sent us a block we clearly didn't
 		// request, we disconnect.
 		if block.Height > pool.height || block.Height < pool.startHeight {
-			err := fmt.Errorf("peer sent us block #%d we didn't expect (current height: %d, start height: %d)",
+			sendErr = fmt.Errorf("peer sent us block #%d we didn't expect (current height: %d, start height: %d)",
 				block.Height, pool.height, pool.startHeight)
-			pool.sendError(err, peerID)
-			return err
+			return sendErr
 		}
 
 		return fmt.Errorf("got an already committed block #%d (possibly from the slow peer %s)", block.Height, peerID)
 	}
 
 	if !requester.setBlock(block, extCommit, peerID) {
-		err := fmt.Errorf("requested block #%d from %v, not %s", block.Height, requester.requestedFrom(), peerID)
-		pool.sendError(err, peerID)
-		return err
+		sendErr = fmt.Errorf("requested block #%d from %v, not %s", block.Height, requester.requestedFrom(), peerID)
+		return sendErr
 	}
 
 	pool.numPending.Add(-1)
diff --git a/blocksync/pool_test.go b/blocksync/pool_test.go
index 8c3b77843a..be0cf8749f 100644
--- a/blocksync/pool_test.go
+++ b/blocksync/pool_test.go
@@ -640,6 +640,46 @@ func TestBlockPoolMaxPeerHeightRefreshesOnPopRequest(t *testing.T) {
 		"peer B must contribute to maxPeerHeight once pool.height reaches its base")
 }
 
+// TestAddBlockDoesNotDeadlockOnSendError is a regression test for AddBlock
+// holding pool.mtx while calling sendError on an unbuffered channel.
+func TestAddBlockDoesNotDeadlockOnSendError(t *testing.T) {
+	requestsCh := make(chan BlockRequest, 10)
+	errorsCh := make(chan peerError) // unbuffered: keeps AddBlock blocked in sendError
+
+	pool := NewBlockPool(1, requestsCh, errorsCh, time.Second)
+	pool.SetLogger(log.TestingLogger())
+	require.NoError(t, pool.Start())
+	t.Cleanup(func() { _ = pool.Stop() })
+
+	pool.mtx.Lock()
+	req := newBPRequester(pool, 1)
+	req.peerID = "A"
+	pool.requesters[1] = req
+	pool.mtx.Unlock()
+
+	block := &types.Block{Header: types.Header{Height: 1}, LastCommit: &types.Commit{}}
+	extCommit := &types.ExtendedCommit{Height: 1}
+
+	// "B" did not request the block; setBlock fails → sendError while holding pool.mtx.
+	go func() { _ = pool.AddBlock("B", block, extCommit, 123) }()
+	time.Sleep(50 * time.Millisecond)
+
+	heightDone := make(chan struct{})
+	go func() {
+		pool.Height()
+		close(heightDone)
+	}()
+
+	select {
+	case <-heightDone:
+		<-errorsCh
+	case <-time.After(500 * time.Millisecond):
+		<-errorsCh
+		<-heightDone
+		t.Fatal("deadlock: AddBlock held pool.mtx while blocked in sendError")
+	}
+}
+
 func TestBlockPoolHasPendingRequestFrom(t *testing.T) {
 	requestsCh := make(chan BlockRequest, 10)
 	errorsCh := make(chan peerError, 10)