fix(core): Fix checkpoint Drop stack overflow

Fixes #1634

diff --git a/crates/core/src/checkpoint.rs b/crates/core/src/checkpoint.rs
index 23d5731f9bb89ddbdf563d94f994d16e98b1ce28..9b5b0bbd46e766eff6672542cdba972b6d427d6f 100644 (file)
--- a/crates/core/src/checkpoint.rs
+++ b/crates/core/src/checkpoint.rs
@@ -21,6 +21,34 @@ struct CPInner {
     prev: Option<Arc<CPInner>>,
 }
 
+/// 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());