Skip to content

refactor(mempool): make mempool lock poisonable#2103

Merged
kkovaacs merged 6 commits into
nextfrom
krisztian/mempool-poison
May 21, 2026
Merged

refactor(mempool): make mempool lock poisonable#2103
kkovaacs merged 6 commits into
nextfrom
krisztian/mempool-poison

Conversation

@kkovaacs
Copy link
Copy Markdown
Contributor

Currently if a mempool operation panics, its possible to leave the mempool in a corrupted state. This PR changes mempool locking to poisonable, meaning that after a panic with the mempool lock held the corrupted mempool cannot be accessed. (For example, if add transaction panics, then subsequent block and batch building should not proceed.)

Changed behavior:

  • SharedMempool now uses std::sync::Mutex.
  • SharedMempool::lock() returns Result<MutexGuard<_>, MempoolPoisonError>.
  • RPC submit/subscription paths map poison to internal errors.
  • Batch/block builders propagate poison as fatal.
  • Mempool-only helper methods are synchronous; commit_block remains async for store I/O.

Closes #2016

@kkovaacs kkovaacs added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label May 20, 2026
Currently if a mempool operation panics, its possible to leave the
mempool in a corrupted state. This PR changes mempool locking to
poisonable, meaning that after a panic with the mempool lock held the
corrupted mempool cannot be accessed. (For example, if add transaction
panics, then subsequent block and batch building should not proceed.)

Changed behavior:
- `SharedMempool` now uses `std::sync::Mutex`.
- `SharedMempool::lock()` returns `Result<MutexGuard<_>,
  MempoolPoisonError>`.
- RPC submit/subscription paths map poison to internal errors.
- Batch/block builders propagate poison as fatal.
- Mempool-only helper methods are synchronous; `commit_block` remains
  async for store I/O.

Closes #2016
@kkovaacs kkovaacs force-pushed the krisztian/mempool-poison branch from 6f9e774 to ab54543 Compare May 20, 2026 10:07
@kkovaacs kkovaacs marked this pull request as ready for review May 20, 2026 13:35
@kkovaacs kkovaacs force-pushed the krisztian/mempool-poison branch from 47c4a38 to f6d9938 Compare May 20, 2026 13:41
Comment thread crates/block-producer/src/server/mod.rs
Copy link
Copy Markdown
Collaborator

@sergerad sergerad left a comment

Choose a reason for hiding this comment

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

LGTM but I'm wondering about design of the nested mutex (predates this PR).

    mempool: Mutex<SharedMempool>,
// ...
pub struct SharedMempool(Arc<Mutex<Mempool>>);

Comment thread crates/block-producer/src/errors.rs
Comment thread crates/block-producer/src/block_builder/mod.rs Outdated
Comment thread crates/block-producer/src/batch_builder/mod.rs Outdated
@kkovaacs kkovaacs merged commit 5d6772f into next May 21, 2026
20 checks passed
@kkovaacs kkovaacs deleted the krisztian/mempool-poison branch May 21, 2026 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog This PR does not require an entry in the `CHANGELOG.md` file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mempool lock should be poisonable

3 participants