Skip to content

txnprovider/txpool: release the pool lock when a caller stops waiting for a block - #23333

Merged
lystopad merged 2 commits into
mainfrom
feature/lystopad/txpool-best-lock
Aug 17, 2026
Merged

txnprovider/txpool: release the pool lock when a caller stops waiting for a block#23333
lystopad merged 2 commits into
mainfrom
feature/lystopad/txpool-best-lock

Conversation

@lystopad

Copy link
Copy Markdown
Member

TxPool.best takes the pool lock and then waits for the block it was asked to build on top of:

p.lock.Lock()
for last := p.lastSeenBlock.Load(); last < onTopOf; last = p.lastSeenBlock.Load() {
    select {
    case <-ctx.Done():
        return false, 0, ctx.Err()   // <- still holding p.lock
    default:
    }
    p.lastSeenCond.Wait()
}
...
p.lock.Unlock()

A caller that goes away while waiting returns from inside the loop without releasing the lock. The only unlock is past the loop, so the lock stays held for the life of the process and every later pool operation blocks behind it — OnNewBlock, ProvideTxns, AddLocalTxns, and shutdown.

It does not recover on its own: the thing that would let the wait finish is a block update, and that needs the same lock.

Why now

Today this is reachable only at shutdown, because the only caller that cancels this context is the one shutting the node down, which is why it has not been noticed.

It stops being shutdown-only as soon as anything cancels a live build. #23272 gives each payload builder a cancellable context so that discarding an evicted one actually releases its resources, and a builder waiting here is then cancelled during normal operation — a routine eviction would deadlock the pool. @yperbasis found it while reviewing that PR and suggested taking it separately, which is what this is.

Scope

Only the missing unlock. Two things I deliberately did not change:

  • cancelling the context does not wake a goroutine parked in lastSeenCond.Wait(), so an evicted builder still returns at the next block rather than immediately. Making the wait cancellable is a larger change and a separate question from the lock being leaked.
  • the ordering comment below the loop, about p.lock and the poolDB read-transaction limiter, is untouched.

TestBestReleasesTheLockWhenTheCallerGivesUpWaitingForABlock covers it, and fails on the current code with "best returned holding the pool lock".

Note: make lint reports db/seg/decompress.go:199: field residencyOnce is unused, which is pre-existing on main — I confirmed it with this change stashed. golangci-lint is clean for txnprovider/txpool/....

… for a block

best takes the pool lock and then waits for the block it was asked to build on top of. A
caller that goes away in the meantime returned from inside that loop without releasing it, so
the lock stayed held and every later pool operation blocked, including the block updates that
would have let the wait finish. Nothing recovers from that on its own.

Reachable today only at shutdown, because the only caller that cancels is the one shutting the
node down. It stops being shutdown-only as soon as anything cancels a live build, which is
what discarding an evicted payload builder does.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a mutex leak in transaction selection when a waiting caller cancels.

Changes:

  • Unlocks the pool before returning on context cancellation.
  • Adds regression coverage verifying the lock remains usable.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
txnprovider/txpool/pool.go Releases the pool lock on cancellation.
txnprovider/txpool/pool_best_lock_test.go Tests cancellation and lock release.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@lystopad
lystopad added this pull request to the merge queue Aug 17, 2026
Merged via the queue into main with commit a497fb7 Aug 17, 2026
134 checks passed
@lystopad
lystopad deleted the feature/lystopad/txpool-best-lock branch August 17, 2026 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants