Skip to content

fix(database): use conservative block_in_place strategy to prevent deadlock#3251

Merged
rakita merged 1 commit intobluealloy:mainfrom
woojinnn:fix/async-db-runtime-deadlock
Dec 23, 2025
Merged

fix(database): use conservative block_in_place strategy to prevent deadlock#3251
rakita merged 1 commit intobluealloy:mainfrom
woojinnn:fix/async-db-runtime-deadlock

Conversation

@woojinnn
Copy link
Copy Markdown
Contributor

@woojinnn woojinnn commented Dec 23, 2025

Summary

Reverts a broken pointer comparison in HandleOrRuntime::block_on that was intended to optimize different-runtime scenarios but instead reintroduced deadlock risk for the common same-runtime case.

Background: The Original Change

A recent change(#3212) added pointer comparison to detect same-runtime scenarios:

core::ptr::eq(handle as *const _, &current as *const _)

The stated intent was:

"Prevents potential deadlock when WrapDatabaseAsync is created with a handle from a different runtime via with_handle(). The code now verifies that the stored handle belongs to the current runtime before using block_in_place."

The idea was:

  • Same runtime → use block_in_place (prevent deadlock)
  • Different runtime → skip block_in_place (assumed unnecessary)

Problem 1: The Implementation is Broken

The core::ptr::eq comparison always returns false because it compares memory addresses of two different Handle struct instances, not their logical runtime identity:

  • handle is stored in the WrapDatabaseAsync struct (heap address)
  • current is returned by Handle::try_current() (stack address)

Result: block_in_place is NEVER used, even for same-runtime scenarios.

Impact of Broken Code

Scenario Intended Actual Result
Same runtime (common) Use block_in_place Skip block_in_place DEADLOCK RISK
Different runtime (rare) Skip block_in_place Skip block_in_place Works (by accident)

The broken code reintroduces deadlock in the common case while "fixing" an edge case.

This can be reproduced by running cargo run -p example-block-traces. Below is the error log:

Fetched block number: 10889447
Found 195 transactions.

thread 'main' (2616100) panicked at /home/woojin.lee/proj_ethereum/revm/crates/database/interface/src/async_db.rs:268:28:
Cannot start a runtime from within a runtime. This happens because a function (like `block_on`) attempted to block the current thread while the thread is being used to drive asynchronous tasks.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Same for some other examples codes, like cargo run -p example-uniswap-get-reserves.

Problem 2: The Original Concern is Unfounded

The original change assumed that using block_in_place with a different-runtime handle is problematic.

However, block_in_place is about freeing up the current runtime's worker thread, not about which runtime we're blocking on:

Using block_in_place with DIFFERENT runtime handle:

Current Runtime (multi-threaded)     Target Runtime (different)
┌─────────────────────────────┐      ┌─────────────────────────┐
│ Worker Thread               │      │                         │
│   │                         │      │                         │
│   ├─ block_in_place()       │      │                         │
│   │   (frees worker thread) │      │                         │
│   │                         │      │                         │
│   └─► handle.block_on(f) ───┼─────►│ Executes future f       │
│       (blocks on different  │      │                         │
│        runtime - SAFE!)     │      │                         │
│                             │      │                         │
│ ✓ Other tasks continue      │      │                         │
└─────────────────────────────┘      └─────────────────────────┘

Result: No deadlock! Current runtime's worker thread is freed.

Solution

Replace the broken pointer comparison with a conservative approach: use block_in_place for ALL multi-threaded runtime contexts.

let should_use_block_in_place = Handle::try_current()
    .ok()
    .map(|current| {
        !matches!(
            current.runtime_flavor(),
            tokio::runtime::RuntimeFlavor::CurrentThread
        )
    })
    .unwrap_or(false);

The implemented logic is technically sound and follows one of the 'Bridge' patterns recommended by Tokio. Even when calling block_on on a Handle from a different runtime, block_in_place will handle it safely as long as the current thread is a worker in a multi-threaded runtime

Why This is Correct

Scenario Behavior Outcome
Same multi-threaded runtime Uses block_in_place Prevents deadlock
Different multi-threaded runtime Uses block_in_place Safe (frees current worker)
Current-thread runtime Uses block_on directly Correct (block_in_place would panic)
Outside any runtime Uses block_on directly Correct

Related

#1056

   async_db block_in_place usage
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Dec 23, 2025

CodSpeed Performance Report

Merging #3251 will not alter performance

Comparing woojinnn:fix/async-db-runtime-deadlock (a4560b1) with main (cae8dd3)

Summary

✅ 173 untouched

@woojinnn woojinnn marked this pull request as ready for review December 23, 2025 15:30
@rakita rakita merged commit b6cc546 into bluealloy:main Dec 23, 2025
31 checks passed
@github-actions github-actions bot mentioned this pull request Dec 23, 2025
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.

2 participants