Skip to content

Commit e4e2344

Browse files
committed
fix(core/txpool): fix Sync deadlock and lost head events in pool loop
Two CI flakes in the txpool reset synchronization surfaced as a 45-minute test timeout on pool.Sync() and sporadic "nonce too high" rejections. Sync() could block forever: the resetDone branch notified the waiter with a non-blocking send and then unconditionally cleared it. When Sync() had not yet reached its receive (a scheduling gap after it sent its waiter), the notification was dropped, leaving the waiter never signaled. Restore the blocking send used upstream, which cannot lose the notification. Chain head events could be lost: the loop subscribed to head events only after its goroutine started, so an InsertChain event emitted right after New could be missed. The pool then reset against a stale head, keeping pending nonces at genesis and rejecting fresh transactions as ErrNonceTooHigh. Move the subscription into New so events are captured from the start. Stabilize TestResubmit and TestJournal by syncing the pool after Add before reading its content.
1 parent 436a229 commit e4e2344

2 files changed

Lines changed: 34 additions & 23 deletions

File tree

core/txpool/locals/tx_tracker_test.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ func TestResubmit(t *testing.T) {
130130
txsA := txs[:len(txs)/2]
131131
txsB := txs[len(txs)/2:]
132132
env.pool.Add(txsA, true)
133+
if err := env.pool.Sync(); err != nil {
134+
t.Fatalf("Failed to sync the txpool: %v", err)
135+
}
133136

134137
pending, queued := env.pool.ContentFrom(address)
135138
if len(pending) != len(txsA) || len(queued) != 0 {
@@ -163,6 +166,9 @@ func TestJournal(t *testing.T) {
163166
txsA := txs[:len(txs)/2]
164167
txsB := txs[len(txs)/2:]
165168
env.pool.Add(txsA, true)
169+
if err := env.pool.Sync(); err != nil {
170+
t.Fatalf("Failed to sync the txpool: %v", err)
171+
}
166172

167173
pending, queued := env.pool.ContentFrom(address)
168174
if len(pending) != len(txsA) || len(queued) != 0 {

core/txpool/txpool.go

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ type TxPool struct {
7676
quit chan chan error // Quit channel to tear down the head updater
7777
term chan struct{} // Termination channel to detect a closed pool
7878

79+
newHeadCh chan core.ChainHeadEvent // Channel of chain head events to trigger subpool resets
80+
newHeadSub event.Subscription // Subscription to the chain head events
81+
7982
sync chan chan error // Testing / simulator channel to block until internal reset is done
8083
}
8184

@@ -105,13 +108,17 @@ func New(gasTip uint64, chain BlockChain, subpools []SubPool) (*TxPool, error) {
105108
return nil, err
106109
}
107110
pool := &TxPool{
108-
subpools: subpools,
109-
chain: chain,
110-
state: statedb,
111-
quit: make(chan chan error),
112-
term: make(chan struct{}),
113-
sync: make(chan chan error),
111+
subpools: subpools,
112+
chain: chain,
113+
state: statedb,
114+
quit: make(chan chan error),
115+
term: make(chan struct{}),
116+
sync: make(chan chan error),
117+
newHeadCh: make(chan core.ChainHeadEvent),
114118
}
119+
// Subscribe to chain head events synchronously, before any block is inserted,
120+
// so head events emitted right after initialization are not lost.
121+
pool.newHeadSub = chain.SubscribeChainHeadEvent(pool.newHeadCh)
115122
reserver := NewReservationTracker()
116123
for i, subpool := range subpools {
117124
if err := subpool.Init(gasTip, head, reserver.NewHandle(i)); err != nil {
@@ -157,12 +164,11 @@ func (p *TxPool) loop(head *types.Header) {
157164
// Close the termination marker when the pool stops
158165
defer close(p.term)
159166

160-
// Subscribe to chain head events to trigger subpool resets
161-
var (
162-
newHeadCh = make(chan core.ChainHeadEvent)
163-
newHeadSub = p.chain.SubscribeChainHeadEvent(newHeadCh)
164-
)
165-
defer newHeadSub.Unsubscribe()
167+
// Consume chain head events to trigger subpool resets. The subscription is
168+
// established synchronously in New, so head events emitted immediately after
169+
// initialization are delivered instead of being lost.
170+
newHeadCh := p.newHeadCh
171+
defer p.newHeadSub.Unsubscribe()
166172

167173
// Track the previous and current head to feed to an idle reset
168174
var (
@@ -177,13 +183,12 @@ func (p *TxPool) loop(head *types.Header) {
177183
resetForced bool // Whether a forced reset was requested, only used in simulator mode
178184
resetWaiter chan error // Channel waiting on a forced reset, only used in simulator mode
179185
)
180-
// Notify the live reset waiter without blocking if the txpool is closed.
186+
// Notify the live reset waiter when the pool terminates. The send blocks
187+
// until Sync() picks it up; it cannot abort via p.term (closed only after
188+
// this defer runs), so the notification is guaranteed to be delivered.
181189
defer func() {
182190
if resetWaiter != nil {
183-
select {
184-
case resetWaiter <- errors.New("pool already terminated"):
185-
default:
186-
}
191+
resetWaiter <- errors.New("pool already terminated")
187192
resetWaiter = nil
188193
}
189194
}()
@@ -245,12 +250,12 @@ func (p *TxPool) loop(head *types.Header) {
245250
// the forced op is still pending. In that case, wait another round
246251
// of resets.
247252
if resetWaiter != nil && !resetForced {
248-
select {
249-
case resetWaiter <- nil:
250-
// notification delivered
251-
default:
252-
// no active listener; avoid blocking the event loop
253-
}
253+
// Block until the waiter receives the notification. Sync() is
254+
// guaranteed to be waiting on its waiter channel (it cannot
255+
// abort via p.term while this loop is still running), so a
256+
// non-blocking send here could drop the notification and leave
257+
// Sync() blocked forever.
258+
resetWaiter <- nil
254259
resetWaiter = nil
255260
}
256261

0 commit comments

Comments
 (0)