Runtime deadlock in sync `SignerProvider` callbacks

[jkczyz]

Summary

<WalletKeysManager as SignerProvider>::get_shutdown_scriptpubkey (and get_destination_script) call Runtime::block_on(wallet.get_new_address()), which awaits the wallet persister lock and store I/O. LDK invokes these sync callbacks on runtime worker threads from inside the event handler (e.g., ChannelManager::do_accept_inbound_channel → ChannelContext::new_for_inbound_channel), while holding the per-peer channel Mutex. If the block_on future cannot complete without another runtime task being polled, and the remaining worker cores get captured by tasks blocking synchronously on the same channel locks, the runtime deadlocks permanently.

This is the same class as #978 (fixed by #980) but a different entry point: #980 stopped holding the sync BDK wallet lock across store awaits and isolated store I/O onto dedicated runtimes, but the SignerProvider callbacks still block a worker on wallet persistence while holding LDK channel locks — which lets unrelated runtime tasks wedge the scheduler through those locks.

Observed deadlock (thread samples)

Hit 2 of 2 full cargo test --test integration_tests_rust runs (macOS, 2026-07-30), striking whichever test was unlucky under whole-suite load — open_channel_variants_reserve_funds_for_anchor_peers in one run, split_underpaid_bolt11_payment in the other. Both use #[tokio::test(flavor = "multi_thread", worker_threads = 1)], so the node runs on the test's single-worker runtime. Not reproducible standalone without suite load (see deterministic repro below).

macOS sample shows the identical three-party cycle in both hangs:

Thread A — background processor task (holds the per-peer channel Mutex, parked forever):

lightning_background_processor::process_events_async
 ChannelManager::process_pending_events_async
 ldk_node::event::EventHandler::handle_event [OpenChannelRequest]
 ChannelManager::accept_inbound_channel
 ChannelManager::do_accept_inbound_channel <-- holds per-peer state Mutex
 InboundV1Channel::new -> ChannelContext::new_for_inbound_channel
 <SignerProvider>::get_shutdown_scriptpubkey
 ldk_node::runtime::Runtime::block_on
 tokio block_in_place <-- hands its worker core off
 Handle::block_on -> CachedParkThread::park -> pthread_cond_wait

Thread B — the fresh thread that received Thread A's core via the block_in_place handoff (captures the core, blocked without handoff):

tokio worker::run (block_in_place continuation)
 run_task: lightning_net_tokio::Connection::poll_event_process
 PeerManager::process_events
 ChannelManager::get_and_clear_pending_msg_events
 PersistenceNotifierGuard::drop -> maybe_generate_initial_closing_signed
 std::sync::Mutex::lock -> __psynch_mutexwait <-- blocks on Thread A's peer Mutex

Party 3 — the future Thread A waits on. Wallet::get_new_address awaits the wallet persister tokio::sync::Mutex and then persist_changeset (store I/O). Under load that mutex is routinely held by another runtime task (wallet sync / apply_update, block_connected, etc.). That task is queued on the runtime — but the only core is captive on Thread B, so it can never run to release the lock, so Thread A is never woken, so the peer Mutex is never released, so Thread B never unblocks. No party can make progress; the hang is permanent, and even tokio timers stop firing (no worker polls the timer wheel).

worker_threads = 1 is the minimal trigger, but any runtime deadlocks once N cores are simultaneously captured by tasks blocking synchronously on locks held by parked block_on callers.

Deterministic reproduction

Gate a node's bdk_wallet-namespace KVStore::writes (in-memory store whose wallet writes await an already-held RwLock) and have a peer open a channel to it on a worker_threads = 1 runtime. The acceptor's event handler enters get_shutdown_scriptpubkey → block_on → parks on the gated write while holding the peer Mutex; the lightning-net-tokio connection task then captures the handed-off core in maybe_generate_initial_closing_signed; timers die; the node never emits ChannelPending. A thread sample of the wedged test process shows exactly the same stacks as the organic hangs.

(Regression test included in the fix PR: channel_open_completes_while_wallet_persistence_is_stalled.)

Proposed fix

Deriving a fresh address does not need to wait for persistence — only the reveal must be ordered. Make the sync SignerProvider callbacks non-blocking:

• Reveal the next external address under the sync BDK wallet lock (in-memory operation, no awaits).
• Leave the resulting change set staged and flush it from a spawned background task that takes the persister lock and then take_staged() — the same ordering every other persist path uses, so out-of-order last_revealed regressions are impossible.

Semantics change: a persistence failure no longer fails the channel open (previously Err(()) from the callback). If the node crashes before the flush lands, the revealed index is re-revealed after restart, potentially reusing the address; BDK's keychain lookahead still detects funds sent to it. This matches the durability LDK's own KeysManager provides for shutdown scripts (none — it derives a static script and persists nothing).

Related bridges not covered here

• OnchainPayment::new_address and LSPS1Liquidity::request_channel still block_on(get_new_address()), but they are public sync API entry points invoked on user threads and do not hold LDK locks. They are the known/accepted sync-bridge pattern (cf. the StoreRuntime docs), though they remain susceptible if users call them from runtime context.
• <Wallet as Listen>::block_connected also bridges via block_on while holding the wallet persister lock (bitcoind chain source only). It does not hold LDK channel locks, so it cannot form this cycle by itself, but it can be one of the parked parties in a multi-worker capture.
• Long-term, async SignerProvider/ChangeDestinationSource callbacks in LDK would remove the need for the bridge entirely.

Provenance

Introduced by #919 (commit 5ac2ed1, "Use BDK's async wallet persister"): before it, get_new_address persisted through BDK's sync persister with no runtime involvement, so the sync SignerProvider callbacks could never park on the runtime. #919 bridged the still-sync wallet APIs via Runtime::block_on (flagged in its commit message as a stopgap). #980 later reshaped the bridge (moving the block_on into the callbacks themselves) while fixing #978, but this cycle runs through the LDK peer Mutex rather than the wallet lock, so it survived.

Environment

• ldk-node main @ abbed3e, LDK 0.3.0+git (lightningdevkit/rust-lightning@506cb91f)
• macOS (Darwin 25.5.0), Apple Silicon; hit under cargo test whole-suite load

[jkczyz]

@tnull I had Clause draft #1011 as a fix, though I'm not sure if it's the approach we want.

[jkczyz]

I asked Claude to look for anything similarly prone to deadlocks:

I audited the rest of the codebase for other blocking bridges reachable from runtime worker threads (all block_on/block_in_place/blocking_* call sites, sync LDK callback impls, and sync locks held across awaits). Summary: one residual bridge of the same shape, lower risk; everything else checked out clean.

Residual bridge: <Wallet as Listen>::block_connected

This is the one remaining place a sync LDK callback on a runtime worker calls Runtime::block_on (wallet/mod.rs#L1854). It is only reachable with the bitcoind chain source: the async poll task (bitcoind.rs#L414) drives SpvClient::poll_best_tip().await, which dispatches to the sync ChainListener::block_connected (bitcoind.rs#L1478), and the wallet impl parks the worker while awaiting the persister lock, payment-store writes, and the wallet persist.

Why it's lower risk than the SignerProvider case: the callback holds no LDK locks when it parks — the wallet is the first listener in the dispatch sequence, and its own locks are acquired inside the awaited future (the sync wallet guard is scoped and dropped before any await). So it can't form the three-party cycle above on its own; it would only wedge if some other task captured every core while holding what its future needs, and with the SignerProvider fix the known core-capturer (lightning-net-tokio blocking on the peer Mutex) no longer has a parked lock-holder to collide with. Since the caller is already async, the eventual fix is to apply blocks to the wallet through an async path instead of the sync Listen trait.

Related coupling in the same function: it holds the wallet persister lock across the payment-store writes (wallet/mod.rs#L1904), so slow payment-store I/O extends how long every get_new_address bridge waits. There's no ordering inversion (DataStore never touches the wallet persister), so this is latency coupling, not a deadlock.

Checked and clean

• Other sync LDK callbacks: ChangeDestinationSource (wallet/mod.rs#L2119) and WalletSource are fully async; the broadcaster uses a non-blocking try_send (tx_broadcaster.rs#L160); the fee estimator reads a cache; Filter does in-memory inserts; LDK's OutputSweeper Listen impl only mutates in-memory state and defers persistence (sweep.rs#L758). Monitor persistence is fully async via MonitorUpdatingPersisterAsync.
• Store layer: no KVStoreSync anywhere; the SQLite store bridges through spawn_blocking (blocking pool, not worker cores); the VSS store has a dedicated internal runtime and does setup on a separate thread. Store futures can always complete independently of the node runtime.
• Sync guards across awaits: none found — DataStore scopes its std::sync::Mutex guards before every await and serializes mutations with a tokio Mutex (data_store.rs#L48); Wallet::block_connected drops the wallet guard before awaiting.
• Public sync API bridges (start/stop, payment APIs, OnchainPayment::new_address, LSPS1, export_pathfinding_scores): user-thread entry points, i.e. the documented StoreRuntime pattern. If called from inside the node's runtime, block_in_place hands the core off, and the self-referential case (calling stop() from the event handler) degrades to a timeout-and-abort rather than a permanent hang.

Hardening idea (not a bug)

RuntimeSpawner spawns the async monitor-persist futures onto the main node runtime (runtime.rs#L366), so any future runtime wedge also silently stalls monitor persistence. Spawning them on a dedicated runtime (like the store-internal ones) would decouple that.

Audit performed with the assistance of Claude Code.