From 67e1dc37bff195d91f2959f3f4bf5d5d10da597c Mon Sep 17 00:00:00 2001 From: LLFourn Date: Tue, 19 Nov 2024 10:47:40 +1100 Subject: [PATCH] fix(core): Fix checkpoint Drop stack overflow Fixes #1634 --- crates/core/src/checkpoint.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/crates/core/src/checkpoint.rs b/crates/core/src/checkpoint.rs index 23d5731f..9b5b0bbd 100644 --- a/crates/core/src/checkpoint.rs +++ b/crates/core/src/checkpoint.rs @@ -21,6 +21,34 @@ struct CPInner { prev: Option>, } +/// When a `CPInner` is dropped we need to go back down the chain and manually remove any +/// no-longer referenced checkpoints. Letting the default rust dropping mechanism handle this +/// leads to recursive logic and stack overflows +/// +/// https://github.com/bitcoindevkit/bdk/issues/1634 +impl Drop for CPInner { + fn drop(&mut self) { + // Take out `prev` so its `drop` won't be called when this drop is finished + let mut current = self.prev.take(); + while let Some(arc_node) = current { + // Get rid of the Arc around `prev` if we're the only one holding a ref + // So the `drop` on it won't be called when the `Arc` is dropped. + // + // FIXME: When MSRV > 1.70.0 this should use Arc::into_inner which actually guarantees + // that no recursive drop calls can happen even with multiple threads. + match Arc::try_unwrap(arc_node).ok() { + Some(mut node) => { + // Keep goign backwards + current = node.prev.take(); + // Don't call `drop` on `CPInner` since that risks it becoming recursive. + core::mem::forget(node); + } + None => break, + } + } + } +} + impl PartialEq for CheckPoint { fn eq(&self, other: &Self) -> bool { let self_cps = self.iter().map(|cp| cp.block_id()); -- 2.49.0