Block engine self-deadlocks when the runtime shuts down with a reclaim in flight

Open

huyz0opened 10d ago

What happens

BlockManager::reclaim_if_needed spawns the reclaim future while the caller holds the state write lock (foyer-storage/src/engine/block/manager.rs):

fn reclaim_if_needed(&self, state: &mut RwLockWriteGuard<'_, State>) {
    if state.clean_blocks.len() < self.inner.clean_block_threshold
        && state.reclaiming_blocks.len() < self.inner.reclaim_concurrency
        && let Some(block) = self.evict(state)
    {
        // ...
        let block = ReclaimingBlock { block_manager: self.clone(), block };
        let future = self.inner.reclaimer.reclaim(block);
        self.inner.spawner.spawn(future);   // <- lock still held
    }
}

Once the tokio runtime has begun shutting down, spawn does not queue the task: tokio cancels it immediately, on the calling thread (OwnedTasks::bind_inner â†’ Task::shutdown). Cancelling it drops its ReclaimingBlock, whose Drop calls on_reclaim_finish, which takes self.inner.state.write() again. std::sync::RwLock is not reentrant, so the thread waits on itself forever.

Shutdown itself starts the cycle: the runtime cancels an in-flight reclaim task, its ReclaimingBlock::drop runs on_reclaim_finish, which calls reclaim_if_needed with the lock held, which spawns onto the closed runtime.

Calling HybridCache::close() before the runtime shuts down should avoid this, but close() cannot run on every path (a panic, an early ? return, or Drop, which cannot await it). On those paths the expected worst case is a cold cache, not a hung thread.

Reproduction

Default spawner (the current runtime), a 1 MiB disk with 64 KiB blocks, and enough inserts to keep reclaims running. The runtime is then dropped without close():

[dependencies]
foyer = "=0.22.6"
tempfile = "3"
tokio = { version = "1", features = ["rt-multi-thread", "macros", "time"] }

use std::{sync::mpsc, thread, time::Duration};

use foyer::{BlockEngineConfig, DeviceBuilder, FsDeviceBuilder, HybridCache, HybridCacheBuilder, HybridCachePolicy};

fn main() {
    for i in 0..500 {
        let dir = tempfile::tempdir().unwrap();
        let path = dir.path().to_path_buf();
        let rt = tokio::runtime::Builder::new_multi_thread()
            .worker_threads(2)
            .enable_all()
            .build()
            .unwrap();

        rt.block_on(async move {
            let device = FsDeviceBuilder::new(&path).with_capacity(1024 * 1024).build().unwrap();
            let cache: HybridCache<u64, Vec<u8>> = HybridCacheBuilder::new()
                .with_policy(HybridCachePolicy::WriteOnInsertion)
                .memory(64 * 1024)
                .with_weighter(|_, v: &Vec<u8>| v.len())
                .storage()
                .with_engine_config(
                    BlockEngineConfig::new(device)
                        .with_block_size(64 * 1024)
                        .with_clean_block_threshold(4),
                )
                .build()
                .await
                .unwrap();
            for k in 0..2000u64 {
                cache.insert(k, vec![k as u8; 4 * 1024]);
                if k % 100 == 0 {
                    tokio::task::yield_now().await;
                }
            }
            tokio::time::sleep(Duration::from_millis(i % 5)).await;
            // No `cache.close()`, as on a panic or early return.
        });

        let (tx, rx) = mpsc::channel();
        thread::spawn(move || {
            drop(rt);
            let _ = tx.send(());
        });
        if rx.recv_timeout(Duration::from_secs(10)).is_err() {
            eprintln!("iteration {i}: dropping the runtime never returned");
            std::process::exit(1);
        }
    }
    eprintln!("no hang");
}

Result (Rust 1.98.1, tokio 1.54.0): in 5 of 5 runs, dropping the runtime hung on iteration 0 or 1.

To confirm the cause, I added a check at the top of on_reclaim_finish that prints a backtrace when it is entered from inside the spawner.spawn call in reclaim_if_needed. It fired on the hanging iteration (frames trimmed):

foyer_storage::engine::block::manager::BlockManager::on_reclaim_finish       <- blocks on state.write()
<foyer_storage::engine::block::manager::ReclaimingBlock as Drop>::drop
tokio::runtime::task::harness::cancel_task
tokio::runtime::task::Task::shutdown
tokio::runtime::task::list::OwnedTasks::bind_inner                           (runtime closed)
tokio::runtime::scheduler::multi_thread::handle::Handle::bind_new_task
foyer_storage::engine::block::manager::BlockManager::reclaim_if_needed       (spawn, lock held)
foyer_storage::engine::block::manager::BlockManager::on_reclaim_finish       <- holds state.write()
<foyer_storage::engine::block::manager::ReclaimingBlock as Drop>::drop
tokio::runtime::task::harness::cancel_task                                   (runtime shutdown cancels an in-flight reclaim)

Suggested fix

Return the future from reclaim_if_needed instead of spawning it, and have each caller (get_clean_block, on_writing_finish, on_reclaim_finish) release the guard before spawning:

#[must_use]
fn reclaim_if_needed(&self, state: &mut RwLockWriteGuard<'_, State>) -> Option<BoxFuture<'static, ()>> {
    // same checks as today, then:
    //     return Some(self.inner.reclaimer.reclaim(block));
    None
}

// in each caller:
let reclaim = self.reclaim_if_needed(&mut state);
drop(state);
if let Some(future) = reclaim {
    self.inner.spawner.spawn(future);
}

With this change applied to a local copy of foyer-storage 0.22.6, the reproduction above ran all 500 iterations without hanging.

#1374 (fix for #1373) adds two more calls to reclaim_if_needed with the lock held, in init and in get_clean_block when a flusher has to wait, so both would need the same treatment.
#1345 fixes the same pattern (releasing a lock before spawning) in foyer-memory.

Workaround

Give the store its own runtime through with_spawner(Handle) and shut that runtime down with shutdown_background() when the store is dropped. The deadlock can then still happen, but only on one of that runtime's worker threads, which nothing joins, so it no longer hangs the application's own threads.

Activity

No activity yet.