From 65a07a420c562c5bec8fa9b1b3c7330dfecbae91 Mon Sep 17 00:00:00 2001 From: Daniel Liu <139250065@qq.com> Date: Tue, 28 Jul 2026 07:37:33 +0800 Subject: [PATCH] fix(miner): fix timer handshake deadlock in `worker.update()` The mining timer was owned by a dedicated goroutine that exchanged notifications with the update loop over two buffered channels: the loop sent the next duration on resetCh and the goroutine reported expiries on c. Both channels have a capacity of one, so once both buffers were full the two goroutines blocked on each other forever. When that happened the worker stopped draining chainHeadCh and chainSideCh. Chain events are posted synchronously, so block insertion blocked in event.Feed.Send, the downloader never finished its sync round, and the node stopped importing blocks entirely. Own the timer from the update loop itself, which removes the handshake and therefore the cycle. --- miner/worker.go | 53 ++++++++++--------------- miner/worker_test.go | 92 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 33 deletions(-) diff --git a/miner/worker.go b/miner/worker.go index 25b91baa2303..3ce671b64e24 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -144,7 +144,6 @@ type worker struct { chainHeadSub event.Subscription chainSideCh chan core.ChainSideEvent chainSideSub event.Subscription - resetCh chan time.Duration // Channel to request timer resets wg sync.WaitGroup @@ -194,7 +193,6 @@ func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus txsCh: make(chan core.NewTxsEvent, txChanSize), chainHeadCh: make(chan core.ChainHeadEvent, chainHeadChanSize), chainSideCh: make(chan core.ChainSideEvent, chainSideChanSize), - resetCh: make(chan time.Duration, 1), chainDb: eth.ChainDb(), recv: make(chan *Result, resultQueueSize), chain: eth.BlockChain(), @@ -387,58 +385,49 @@ func (w *worker) update() { timeout := time.NewTimer(time.Duration(minePeriod) * time.Second) defer timeout.Stop() - c := make(chan struct{}, 1) - defer close(c) - finish := make(chan struct{}) - defer close(finish) - - go func() { - for { - // A real event arrived, process interesting content + + // resetTimer rearms the mining timer, which is owned by the update loop and + // therefore never accessed from another goroutine. It must only ever be + // called from the loop below. Driving the timer from a dedicated goroutine + // would require a two-way channel handshake, which deadlocks as soon as + // both directions are full: the loop blocks handing over the new duration + // while the helper blocks handing over the expiry notification. The + // stop-drain-reset sequence follows the standard Timer.Reset pattern from + // the time package. + resetTimer := func(d time.Duration) { + if !timeout.Stop() { + // Drain the timer channel if it had already expired. select { - case d := <-w.resetCh: - // Reset the timer to the new duration. - if !timeout.Stop() { - // Drain the timer channel if it had already expired. - select { - case <-timeout.C: - default: - } - } - timeout.Reset(d) case <-timeout.C: - c <- struct{}{} - case <-finish: - return + default: } } - }() + timeout.Reset(d) + } + for { // A real event arrived, process interesting content select { case v := <-minePeriodCh: log.Info("[worker] update wait period", "period", v) minePeriod = v - w.resetCh <- time.Duration(minePeriod) * time.Second + resetTimer(time.Duration(minePeriod) * time.Second) - case <-c: + case <-timeout.C: if atomic.LoadInt32(&w.mining) == 1 { w.commitNewWork() } - resetTime := getResetTime(w.chain, minePeriod) - w.resetCh <- resetTime + resetTimer(getResetTime(w.chain, minePeriod)) // Handle ChainHeadEvent case <-w.chainHeadCh: w.commitNewWork() - resetTime := getResetTime(w.chain, minePeriod) - w.resetCh <- resetTime + resetTimer(getResetTime(w.chain, minePeriod)) // Handle new round case <-newRoundCh: w.commitNewWork() - resetTime := getResetTime(w.chain, minePeriod) - w.resetCh <- resetTime + resetTimer(getResetTime(w.chain, minePeriod)) // Handle ChainSideEvent case <-w.chainSideCh: diff --git a/miner/worker_test.go b/miner/worker_test.go index 82c92cf9844d..0dd71d1cadf5 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -50,7 +50,6 @@ func TestWorkerUpdateNonXDPoSStaysRunning(t *testing.T) { engine: ethash.NewFaker(), chainHeadSub: newBlockingSubscription(), chainSideSub: newBlockingSubscription(), - resetCh: make(chan time.Duration, 1), } done := make(chan struct{}) @@ -83,6 +82,97 @@ func TestWorkerUpdateNonXDPoSStaysRunning(t *testing.T) { } } +// TestWorkerUpdateKeepsDrainingChainHead ensures the mining timer never stops +// the update loop from servicing chain events. +// +// The timer used to be owned by a dedicated goroutine that exchanged +// notifications with the update loop over two buffered channels: the loop sent +// the next duration on resetCh and the goroutine reported expiries on c. Once +// both buffers were full the two goroutines blocked on each other forever. The +// worker then stopped draining chainHeadCh, which in turn blocked every +// producer of chain events (block insertion posts them synchronously) and +// wedged the whole node. +func TestWorkerUpdateKeepsDrainingChainHead(t *testing.T) { + chainConfig := ¶ms.ChainConfig{ + ChainID: big.NewInt(1338), + HomesteadBlock: new(big.Int), + Ethash: new(params.EthashConfig), + } + genesis := &core.Genesis{ + Config: chainConfig, + // Anchor the genesis at the current time so getResetTime returns a + // positive duration and the mining timer stays armed throughout the + // test: block time plus the mine period starts just ahead of "now", + // letting the timer expiry path run while the loop keeps servicing + // chain head events. + Timestamp: uint64(time.Now().Unix()), + Difficulty: big.NewInt(1), + GasLimit: params.XDCGenesisGasLimit, + } + db := rawdb.NewMemoryDatabase() + engine := ethash.NewFaker() + chain, err := core.NewBlockChain(db, nil, genesis, engine, vm.Config{}) + if err != nil { + t.Fatalf("failed to create blockchain: %v", err) + } + defer chain.Stop() + + head := chain.GetBlockByNumber(0) + if head == nil { + t.Fatal("expected genesis block") + } + + // announceTxs is false and mining is 0, so commitNewWork bails out early in + // checkPreCommit and the loop stays cheap. + worker := &worker{ + chainConfig: chainConfig, + engine: engine, + chain: chain, + chainHeadCh: make(chan core.ChainHeadEvent, chainHeadChanSize), + chainSideCh: make(chan core.ChainSideEvent, chainSideChanSize), + chainHeadSub: newBlockingSubscription(), + chainSideSub: newBlockingSubscription(), + } + + done := make(chan struct{}) + go func() { + worker.update() + close(done) + }() + + // drainTimeout bounds how long a single send may block before the test + // concludes that worker.update stopped draining chain events. It is reused + // across iterations instead of calling time.After per send, which would + // allocate a fresh timer on every iteration. + drainTimeout := time.NewTimer(time.Second) + defer drainTimeout.Stop() + + deadline := time.Now().Add(3 * time.Second) + for time.Now().Before(deadline) { + select { + case worker.chainHeadCh <- core.ChainHeadEvent{Block: head}: + // Rearm the timeout for the next send, draining the channel first + // if the timer already expired. + if !drainTimeout.Stop() { + select { + case <-drainTimeout.C: + default: + } + } + drainTimeout.Reset(time.Second) + case <-drainTimeout.C: + t.Fatal("worker.update stopped draining chainHeadCh") + } + } + + worker.chainHeadSub.Unsubscribe() + select { + case <-done: + case <-time.After(time.Second): + t.Fatal("worker.update did not return after unsubscribe") + } +} + // TestWorkerUpdateNewTxsWithoutTRC21Issuer tests worker update new txs without trc 21 issuer. func TestWorkerUpdateNewTxsWithoutTRC21Issuer(t *testing.T) { key, err := crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")