From 1d06f1e7667d3f9c9761161cbbf9e47aa3955559 Mon Sep 17 00:00:00 2001
From: "Kuangyuan.Sun" <kuangyuan.sun@tusimple.ai>
Date: Wed, 25 Jan 2023 22:44:43 -0800
Subject: [PATCH 1/5] try fixing stack overflow issue for large tree

---
 gtsam/inference/BayesTreeCliqueBase.h | 25 ++++++++++++++++++++++++-
 gtsam/inference/ClusterTree.h         | 26 +++++++++++++++++++++++++-
 gtsam/inference/EliminationTree.h     | 24 ++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/gtsam/inference/BayesTreeCliqueBase.h b/gtsam/inference/BayesTreeCliqueBase.h
index 976d1a7f2..f72412590 100644
--- a/gtsam/inference/BayesTreeCliqueBase.h
+++ b/gtsam/inference/BayesTreeCliqueBase.h
@@ -25,6 +25,8 @@
 #include <string>
 #include <mutex>
 #include <optional>
+#include <queue>
+#include <deque>
 
 namespace gtsam {
 
@@ -97,7 +99,28 @@ namespace gtsam {
     }
 
     // Virtual destructor.
-    virtual ~BayesTreeCliqueBase() {}
+    virtual ~BayesTreeCliqueBase() {
+      // default destructor will cause stack overflow for deletion of large trees
+      // so we delete the tree explicitly by first BFSing the tree to determine the topological order
+      // and then clearing the children of each node in topological order
+      // Only work in single threaded mode
+      std::queue<This*> bfs_queue;
+      std::deque<This*> topological_order;
+      bfs_queue.push(this);
+      while (!bfs_queue.empty()) {
+        auto node = bfs_queue.front();
+        bfs_queue.pop();
+        topological_order.push_front(node);
+        for (auto& child : node->children) {
+          if (child.use_count() == 1) {
+            bfs_queue.push(child.get());
+          }
+        }
+      }
+      for (auto node : topological_order) {
+        node->children.clear();
+      }
+    }
 
     /// @}
 
diff --git a/gtsam/inference/ClusterTree.h b/gtsam/inference/ClusterTree.h
index 98a86b3f8..a517951a0 100644
--- a/gtsam/inference/ClusterTree.h
+++ b/gtsam/inference/ClusterTree.h
@@ -9,6 +9,8 @@
 
 #pragma once
 
+#include <deque>
+#include <queue>
 #include <gtsam/base/Testable.h>
 #include <gtsam/base/FastVector.h>
 #include <gtsam/inference/Ordering.h>
@@ -46,7 +48,8 @@ class ClusterTree {
 
     Cluster() : problemSize_(0) {}
 
-    virtual ~Cluster() {}
+    virtual ~Cluster() {
+    }
 
     const Cluster& operator[](size_t i) const {
       return *(children[i]);
@@ -164,6 +167,27 @@ class ClusterTree {
     return *(roots_[i]);
   }
 
+  virtual ~ClusterTree() {
+    // use default destructor which recursively deletes all nodes with shared_ptr causes stack overflow.
+    // so for each tree, we do a BFS to get sequence of nodes to delete, and clear their children first.
+    for (auto&& root : roots_) {
+      std::queue<Cluster*> q;
+      std::deque<Cluster*> nodes;
+      q.push(root.get());
+      while (!q.empty()) {
+        auto node = q.front();
+        nodes.push_front(node);
+        q.pop();
+        for (auto&& child : node->children) {
+          q.push(child.get());
+        }
+      }
+      for (auto&& node : nodes) {
+        node->children.clear();
+      }
+    }
+  }
+
   /// @}
 
  protected:
diff --git a/gtsam/inference/EliminationTree.h b/gtsam/inference/EliminationTree.h
index 2a3abdcf6..dcd561fd2 100644
--- a/gtsam/inference/EliminationTree.h
+++ b/gtsam/inference/EliminationTree.h
@@ -19,6 +19,8 @@
 
 #include <utility>
 #include <memory>
+#include <deque>
+#include <queue>
 
 #include <gtsam/base/Testable.h>
 #include <gtsam/base/FastVector.h>
@@ -118,6 +120,27 @@ namespace gtsam {
     /// @}
 
   public:
+    ~EliminationTree() {
+      // default destructor will cause stack overflow for deletion of large trees
+      // so we delete the tree explicitly by first BFSing the tree to determine the topological order
+      // and then clearing the children of each node in topological order
+      for (auto&& root : roots_) {
+        std::queue<Node*> bfs_queue;
+        std::deque<Node*> topological_order;
+        bfs_queue.push(root.get());
+        while (!bfs_queue.empty()) {
+          Node* node = bfs_queue.front();
+          bfs_queue.pop();
+          topological_order.push_front(node);
+          for (auto&& child : node->children) {
+            bfs_queue.push(child.get());
+          }
+        }
+        for (auto&& node : topological_order) {
+          node->children.clear();
+        }
+      }
+    }
     /// @name Standard Interface
     /// @{
 
@@ -156,6 +179,7 @@ namespace gtsam {
     /** Swap the data of this tree with another one, this operation is very fast. */
     void swap(This& other);
 
+
   protected:
     /// Protected default constructor
     EliminationTree() {}

From e6f2cd49893448e6fea5b085ab9c6a093038e826 Mon Sep 17 00:00:00 2001
From: "Kuangyuan.Sun" <kuangyuan.sun@tusimple.ai>
Date: Thu, 26 Jan 2023 14:35:29 -0800
Subject: [PATCH 2/5] ent

---
 gtsam/inference/BayesTree.h           | 21 +++++++++++++++++++++
 gtsam/inference/BayesTreeCliqueBase.h | 26 ++------------------------
 gtsam/inference/ClusterTree.h         | 18 +++++++++---------
 3 files changed, 32 insertions(+), 33 deletions(-)

diff --git a/gtsam/inference/BayesTree.h b/gtsam/inference/BayesTree.h
index 85bc710b1..6637370cc 100644
--- a/gtsam/inference/BayesTree.h
+++ b/gtsam/inference/BayesTree.h
@@ -27,6 +27,8 @@
 #include <gtsam/base/FastVector.h>
 
 #include <string>
+#include <queue>
+#include <deque>
 
 namespace gtsam {
 
@@ -111,6 +113,25 @@ namespace gtsam {
     /** Copy constructor */
     BayesTree(const This& other);
 
+    ~BayesTree() {
+      for (auto&& root: roots_) {
+        std::queue<Clique*> bfs_queue;
+        std::deque<Clique*> topological_order;
+        bfs_queue.push(root.get());
+        while (!bfs_queue.empty()) {
+          Clique* current = bfs_queue.front();
+          bfs_queue.pop();
+          topological_order.push_front(current);
+          for (auto&& child: current->children) {
+            bfs_queue.push(child.get());
+          }
+        }
+        for (auto&& clique: topological_order) {
+          clique->children.clear();
+        }
+      }
+    }
+
     /// @}
 
     /** Assignment operator */
diff --git a/gtsam/inference/BayesTreeCliqueBase.h b/gtsam/inference/BayesTreeCliqueBase.h
index f72412590..6e50182c4 100644
--- a/gtsam/inference/BayesTreeCliqueBase.h
+++ b/gtsam/inference/BayesTreeCliqueBase.h
@@ -25,8 +25,7 @@
 #include <string>
 #include <mutex>
 #include <optional>
-#include <queue>
-#include <deque>
+
 
 namespace gtsam {
 
@@ -99,28 +98,7 @@ namespace gtsam {
     }
 
     // Virtual destructor.
-    virtual ~BayesTreeCliqueBase() {
-      // default destructor will cause stack overflow for deletion of large trees
-      // so we delete the tree explicitly by first BFSing the tree to determine the topological order
-      // and then clearing the children of each node in topological order
-      // Only work in single threaded mode
-      std::queue<This*> bfs_queue;
-      std::deque<This*> topological_order;
-      bfs_queue.push(this);
-      while (!bfs_queue.empty()) {
-        auto node = bfs_queue.front();
-        bfs_queue.pop();
-        topological_order.push_front(node);
-        for (auto& child : node->children) {
-          if (child.use_count() == 1) {
-            bfs_queue.push(child.get());
-          }
-        }
-      }
-      for (auto node : topological_order) {
-        node->children.clear();
-      }
-    }
+    virtual ~BayesTreeCliqueBase() {}
 
     /// @}
 
diff --git a/gtsam/inference/ClusterTree.h b/gtsam/inference/ClusterTree.h
index a517951a0..63a1b5a96 100644
--- a/gtsam/inference/ClusterTree.h
+++ b/gtsam/inference/ClusterTree.h
@@ -171,18 +171,18 @@ class ClusterTree {
     // use default destructor which recursively deletes all nodes with shared_ptr causes stack overflow.
     // so for each tree, we do a BFS to get sequence of nodes to delete, and clear their children first.
     for (auto&& root : roots_) {
-      std::queue<Cluster*> q;
-      std::deque<Cluster*> nodes;
-      q.push(root.get());
-      while (!q.empty()) {
-        auto node = q.front();
-        nodes.push_front(node);
-        q.pop();
+      std::queue<Cluster*> bfs_queue;
+      std::deque<Cluster*> topological_order;
+      bfs_queue.push(root.get());
+      while (!bfs_queue.empty()) {
+        auto node = bfs_queue.front();
+        topological_order.push_front(node);
+        bfs_queue.pop();
         for (auto&& child : node->children) {
-          q.push(child.get());
+          bfs_queue.push(child.get());
         }
       }
-      for (auto&& node : nodes) {
+      for (auto&& node : topological_order) {
         node->children.clear();
       }
     }

From 53a1c76818dea6d55c510c79b29fd01b746142cd Mon Sep 17 00:00:00 2001
From: "Kuangyuan.Sun" <kuangyuan.sun@tusimple.ai>
Date: Thu, 2 Feb 2023 20:04:27 +0000
Subject: [PATCH 3/5] a better solution for ISUEE-1433

---
 gtsam/inference/BayesTree.h           | 24 +++++++++++++-----------
 gtsam/inference/BayesTreeCliqueBase.h |  1 -
 gtsam/inference/ClusterTree.h         | 22 +++++++++-------------
 gtsam/inference/EliminationTree.h     | 22 +++++++++-------------
 4 files changed, 31 insertions(+), 38 deletions(-)

diff --git a/gtsam/inference/BayesTree.h b/gtsam/inference/BayesTree.h
index 6637370cc..ebca5de25 100644
--- a/gtsam/inference/BayesTree.h
+++ b/gtsam/inference/BayesTree.h
@@ -28,7 +28,6 @@
 
 #include <string>
 #include <queue>
-#include <deque>
 
 namespace gtsam {
 
@@ -114,21 +113,24 @@ namespace gtsam {
     BayesTree(const This& other);
 
     ~BayesTree() {
+      nodes_.clear();  // clear the map manually, as we cannot control the order of destruction in the map
       for (auto&& root: roots_) {
-        std::queue<Clique*> bfs_queue;
-        std::deque<Clique*> topological_order;
-        bfs_queue.push(root.get());
+        std::queue<sharedClique> bfs_queue;
+        
+        // first, move the root to the queue
+        bfs_queue.push(root);
+        root = nullptr;
+
+        // do a BFS on the tree, for each node, add its children to the queue, and then delete it from the queue
+        // So if the reference count of the node is 1, it will be deleted, and because its children are in the queue,
+        // the deletion of the node will not trigger a recursive deletion of the children.
         while (!bfs_queue.empty()) {
-          Clique* current = bfs_queue.front();
+          auto current = bfs_queue.front();
           bfs_queue.pop();
-          topological_order.push_front(current);
-          for (auto&& child: current->children) {
-            bfs_queue.push(child.get());
+          for (auto child: current->children) {
+            bfs_queue.push(child);
           }
         }
-        for (auto&& clique: topological_order) {
-          clique->children.clear();
-        }
       }
     }
 
diff --git a/gtsam/inference/BayesTreeCliqueBase.h b/gtsam/inference/BayesTreeCliqueBase.h
index 6e50182c4..976d1a7f2 100644
--- a/gtsam/inference/BayesTreeCliqueBase.h
+++ b/gtsam/inference/BayesTreeCliqueBase.h
@@ -26,7 +26,6 @@
 #include <mutex>
 #include <optional>
 
-
 namespace gtsam {
 
   // Forward declarations
diff --git a/gtsam/inference/ClusterTree.h b/gtsam/inference/ClusterTree.h
index 63a1b5a96..c724adf5e 100644
--- a/gtsam/inference/ClusterTree.h
+++ b/gtsam/inference/ClusterTree.h
@@ -9,7 +9,6 @@
 
 #pragma once
 
-#include <deque>
 #include <queue>
 #include <gtsam/base/Testable.h>
 #include <gtsam/base/FastVector.h>
@@ -48,8 +47,7 @@ class ClusterTree {
 
     Cluster() : problemSize_(0) {}
 
-    virtual ~Cluster() {
-    }
+    virtual ~Cluster() {}
 
     const Cluster& operator[](size_t i) const {
       return *(children[i]);
@@ -169,22 +167,20 @@ class ClusterTree {
 
   virtual ~ClusterTree() {
     // use default destructor which recursively deletes all nodes with shared_ptr causes stack overflow.
-    // so for each tree, we do a BFS to get sequence of nodes to delete, and clear their children first.
+    // so for each tree, we first move the root into a queue; then we do a BFS on the tree with the queue;
+    // for each node iterated, if its reference count is 1, it will be deleted while its children are still in the queue.
+    // so that the recursive deletion will not happen.
     for (auto&& root : roots_) {
-      std::queue<Cluster*> bfs_queue;
-      std::deque<Cluster*> topological_order;
-      bfs_queue.push(root.get());
+      std::queue<sharedNode> bfs_queue;
+      bfs_queue.push(root);
+      root = nullptr;
       while (!bfs_queue.empty()) {
         auto node = bfs_queue.front();
-        topological_order.push_front(node);
         bfs_queue.pop();
-        for (auto&& child : node->children) {
-          bfs_queue.push(child.get());
+        for (auto child : node->children) {
+          bfs_queue.push(child);
         }
       }
-      for (auto&& node : topological_order) {
-        node->children.clear();
-      }
     }
   }
 
diff --git a/gtsam/inference/EliminationTree.h b/gtsam/inference/EliminationTree.h
index dcd561fd2..404f4f38f 100644
--- a/gtsam/inference/EliminationTree.h
+++ b/gtsam/inference/EliminationTree.h
@@ -121,24 +121,21 @@ namespace gtsam {
 
   public:
     ~EliminationTree() {
-      // default destructor will cause stack overflow for deletion of large trees
-      // so we delete the tree explicitly by first BFSing the tree to determine the topological order
-      // and then clearing the children of each node in topological order
+      // use default destructor which recursively deletes all nodes with shared_ptr causes stack overflow.
+      // so for each tree, we first move the root into a queue; then we do a BFS on the tree with the queue;
+      // for each node iterated, if its reference count is 1, it will be deleted while its children are still in the queue.
+      // so that the recursive deletion will not happen.
       for (auto&& root : roots_) {
-        std::queue<Node*> bfs_queue;
-        std::deque<Node*> topological_order;
-        bfs_queue.push(root.get());
+        std::queue<sharedNode> bfs_queue;
+        bfs_queue.push(root);
+        root = nullptr;
         while (!bfs_queue.empty()) {
-          Node* node = bfs_queue.front();
+          auto node = bfs_queue.front();
           bfs_queue.pop();
-          topological_order.push_front(node);
           for (auto&& child : node->children) {
-            bfs_queue.push(child.get());
+            bfs_queue.push(child);
           }
         }
-        for (auto&& node : topological_order) {
-          node->children.clear();
-        }
       }
     }
     /// @name Standard Interface
@@ -179,7 +176,6 @@ namespace gtsam {
     /** Swap the data of this tree with another one, this operation is very fast. */
     void swap(This& other);
 
-
   protected:
     /// Protected default constructor
     EliminationTree() {}

From 50d3f9c2f5880e1a56339ead676cfc16fb873f03 Mon Sep 17 00:00:00 2001
From: wanmeihuali <ksun6@hotmail.com>
Date: Thu, 2 Feb 2023 14:02:21 -0800
Subject: [PATCH 4/5] Remove useless deps

---
 gtsam/inference/EliminationTree.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/gtsam/inference/EliminationTree.h b/gtsam/inference/EliminationTree.h
index 404f4f38f..fd3eb16f5 100644
--- a/gtsam/inference/EliminationTree.h
+++ b/gtsam/inference/EliminationTree.h
@@ -19,7 +19,6 @@
 
 #include <utility>
 #include <memory>
-#include <deque>
 #include <queue>
 
 #include <gtsam/base/Testable.h>

From 36e9d4aaed05619c780bb2c575f23d16823f397d Mon Sep 17 00:00:00 2001
From: wanmeihuali <ksun6@hotmail.com>
Date: Tue, 7 Feb 2023 17:51:08 -0800
Subject: [PATCH 5/5] move the destructor implementation from .h to -inst.h

---
 gtsam/inference/BayesTree-inst.h       | 40 ++++++++++++++++++++++++++
 gtsam/inference/BayesTree.h            | 26 ++---------------
 gtsam/inference/ClusterTree-inst.h     | 36 +++++++++++++++++++++++
 gtsam/inference/ClusterTree.h          | 22 ++------------
 gtsam/inference/EliminationTree-inst.h | 38 ++++++++++++++++++++++++
 gtsam/inference/EliminationTree.h      | 20 +------------
 6 files changed, 120 insertions(+), 62 deletions(-)

diff --git a/gtsam/inference/BayesTree-inst.h b/gtsam/inference/BayesTree-inst.h
index 2d8f917dc..95d475a02 100644
--- a/gtsam/inference/BayesTree-inst.h
+++ b/gtsam/inference/BayesTree-inst.h
@@ -26,6 +26,7 @@
 #include <gtsam/base/timing.h>
 
 #include <fstream>
+#include <queue>
 
 namespace gtsam {
 
@@ -178,6 +179,45 @@ namespace gtsam {
     *this = other;
   }
 
+  /* ************************************************************************* */
+
+  /** Destructor 
+   * Using default destructor causes stack overflow for large trees due to recursive destruction of nodes;
+   * so we manually decrease the reference count of each node in the tree through a BFS, and the nodes with
+   * reference count 0 will be deleted. Please see [PR-1441](https://github.com/borglab/gtsam/pull/1441) for more details.
+   */
+  template<class CLIQUE>
+  BayesTree<CLIQUE>::~BayesTree() {
+    /* Because tree nodes are hold by both root_ and nodes_, we need to clear nodes_ manually first and
+     * reduce the reference count of each node by 1. Otherwise, the nodes will not be properly deleted
+     * during the BFS process.
+     */
+    nodes_.clear();
+    for (auto&& root: roots_) {
+      std::queue<sharedClique> bfs_queue;
+      
+      // first, move the root to the queue
+      bfs_queue.push(root);
+      root = nullptr; // now the root node is owned by the queue
+
+      // do a BFS on the tree, for each node, add its children to the queue, and then delete it from the queue
+      // So if the reference count of the node is 1, it will be deleted, and because its children are in the queue,
+      // the deletion of the node will not trigger a recursive deletion of the children.
+      while (!bfs_queue.empty()) {
+        // move the ownership of the front node from the queue to the current variable
+        auto current = bfs_queue.front();
+        bfs_queue.pop();
+
+        // add the children of the current node to the queue, so that the queue will also own the children nodes.
+        for (auto child: current->children) {
+          bfs_queue.push(child);
+        } // leaving the scope of current will decrease the reference count of the current node by 1, and if the reference count is 0,
+          // the node will be deleted. Because the children are in the queue, the deletion of the node will not trigger a recursive
+          // deletion of the children.
+      }
+    }
+  }
+
   /* ************************************************************************* */
   namespace {
     template<typename NODE>
diff --git a/gtsam/inference/BayesTree.h b/gtsam/inference/BayesTree.h
index ebca5de25..d0ebd916e 100644
--- a/gtsam/inference/BayesTree.h
+++ b/gtsam/inference/BayesTree.h
@@ -27,7 +27,6 @@
 #include <gtsam/base/FastVector.h>
 
 #include <string>
-#include <queue>
 
 namespace gtsam {
 
@@ -112,30 +111,11 @@ namespace gtsam {
     /** Copy constructor */
     BayesTree(const This& other);
 
-    ~BayesTree() {
-      nodes_.clear();  // clear the map manually, as we cannot control the order of destruction in the map
-      for (auto&& root: roots_) {
-        std::queue<sharedClique> bfs_queue;
-        
-        // first, move the root to the queue
-        bfs_queue.push(root);
-        root = nullptr;
-
-        // do a BFS on the tree, for each node, add its children to the queue, and then delete it from the queue
-        // So if the reference count of the node is 1, it will be deleted, and because its children are in the queue,
-        // the deletion of the node will not trigger a recursive deletion of the children.
-        while (!bfs_queue.empty()) {
-          auto current = bfs_queue.front();
-          bfs_queue.pop();
-          for (auto child: current->children) {
-            bfs_queue.push(child);
-          }
-        }
-      }
-    }
-
     /// @}
 
+    /** Destructor */
+    ~BayesTree(); 
+
     /** Assignment operator */
     This& operator=(const This& other);
 
diff --git a/gtsam/inference/ClusterTree-inst.h b/gtsam/inference/ClusterTree-inst.h
index a6175f76b..318624608 100644
--- a/gtsam/inference/ClusterTree-inst.h
+++ b/gtsam/inference/ClusterTree-inst.h
@@ -18,6 +18,7 @@
 #ifdef GTSAM_USE_TBB
 #include <mutex>
 #endif
+#include <queue>
 
 namespace gtsam {
 
@@ -99,6 +100,41 @@ void ClusterTree<GRAPH>::print(const std::string& s, const KeyFormatter& keyForm
   treeTraversal::PrintForest(*this, s, keyFormatter);
 }
 
+/* ************************************************************************* */
+
+/* Destructor.
+ * Using default destructor causes stack overflow for large trees due to recursive destruction of nodes;
+ * so we manually decrease the reference count of each node in the tree through a BFS, and the nodes with
+ * reference count 0 will be deleted. Please see [PR-1441](https://github.com/borglab/gtsam/pull/1441) for more details.
+ */
+template <class GRAPH>
+ClusterTree<GRAPH>::~ClusterTree() {
+  // For each tree, we first move the root into a queue; then we do a BFS on the tree with the queue;
+  
+  for (auto&& root : roots_) {
+    std::queue<sharedNode> bfs_queue;
+    // first, move the root to the queue
+    bfs_queue.push(root);
+    root = nullptr; // now the root node is owned by the queue
+
+    // for each node iterated, if its reference count is 1, it will be deleted while its children are still in the queue.
+    // so that the recursive deletion will not happen.
+    while (!bfs_queue.empty()) {
+      // move the ownership of the front node from the queue to the current variable
+      auto node = bfs_queue.front();
+      bfs_queue.pop();
+
+      // add the children of the current node to the queue, so that the queue will also own the children nodes.
+      for (auto child : node->children) {
+        bfs_queue.push(child);
+      } // leaving the scope of current will decrease the reference count of the current node by 1, and if the reference count is 0,
+        // the node will be deleted. Because the children are in the queue, the deletion of the node will not trigger a recursive
+        // deletion of the children.
+    }
+  }
+
+}
+
 /* ************************************************************************* */
 template <class GRAPH>
 ClusterTree<GRAPH>& ClusterTree<GRAPH>::operator=(const This& other) {
diff --git a/gtsam/inference/ClusterTree.h b/gtsam/inference/ClusterTree.h
index c724adf5e..db80cee3b 100644
--- a/gtsam/inference/ClusterTree.h
+++ b/gtsam/inference/ClusterTree.h
@@ -9,7 +9,6 @@
 
 #pragma once
 
-#include <queue>
 #include <gtsam/base/Testable.h>
 #include <gtsam/base/FastVector.h>
 #include <gtsam/inference/Ordering.h>
@@ -165,27 +164,10 @@ class ClusterTree {
     return *(roots_[i]);
   }
 
-  virtual ~ClusterTree() {
-    // use default destructor which recursively deletes all nodes with shared_ptr causes stack overflow.
-    // so for each tree, we first move the root into a queue; then we do a BFS on the tree with the queue;
-    // for each node iterated, if its reference count is 1, it will be deleted while its children are still in the queue.
-    // so that the recursive deletion will not happen.
-    for (auto&& root : roots_) {
-      std::queue<sharedNode> bfs_queue;
-      bfs_queue.push(root);
-      root = nullptr;
-      while (!bfs_queue.empty()) {
-        auto node = bfs_queue.front();
-        bfs_queue.pop();
-        for (auto child : node->children) {
-          bfs_queue.push(child);
-        }
-      }
-    }
-  }
-
   /// @}
 
+  ~ClusterTree();
+
  protected:
   /// @name Details
   /// @{
diff --git a/gtsam/inference/EliminationTree-inst.h b/gtsam/inference/EliminationTree-inst.h
index 63cbe222b..e9acc12e1 100644
--- a/gtsam/inference/EliminationTree-inst.h
+++ b/gtsam/inference/EliminationTree-inst.h
@@ -18,6 +18,7 @@
 #pragma once
 
 #include <stack>
+#include <queue>
 
 #include <gtsam/base/timing.h>
 #include <gtsam/base/treeTraversal-inst.h>
@@ -180,6 +181,43 @@ namespace gtsam {
     return *this;
   }
 
+  /* ************************************************************************* */
+  
+  /** Destructor
+   * Using default destructor causes stack overflow for large trees due to recursive destruction of nodes;
+   * so we manually decrease the reference count of each node in the tree through a BFS, and the nodes with
+   * reference count 0 will be deleted. Please see [PR-1441](https://github.com/borglab/gtsam/pull/1441) for more details.
+   */
+  template<class BAYESNET, class GRAPH>
+  EliminationTree<BAYESNET,GRAPH>::~EliminationTree()
+  {
+    // For each tree, we first move the root into a queue; then we do a BFS on the tree with the queue;
+
+    for (auto&& root : roots_) {
+      std::queue<sharedNode> bfs_queue;
+
+      // first, move the root to the queue
+      bfs_queue.push(root);
+      root = nullptr; // now the root node is owned by the queue
+
+      // for each node iterated, if its reference count is 1, it will be deleted while its children are still in the queue.
+      // so that the recursive deletion will not happen.
+      while (!bfs_queue.empty()) {
+        // move the ownership of the front node from the queue to the current variable
+        auto node = bfs_queue.front();
+        bfs_queue.pop();
+
+        // add the children of the current node to the queue, so that the queue will also own the children nodes.
+        for (auto&& child : node->children) {
+          bfs_queue.push(child);
+        } // leaving the scope of current will decrease the reference count of the current node by 1, and if the reference count is 0,
+          // the node will be deleted. Because the children are in the queue, the deletion of the node will not trigger a recursive
+          // deletion of the children.
+      }
+    }
+  }
+
+
   /* ************************************************************************* */
   template<class BAYESNET, class GRAPH>
   std::pair<std::shared_ptr<BAYESNET>, std::shared_ptr<GRAPH> >
diff --git a/gtsam/inference/EliminationTree.h b/gtsam/inference/EliminationTree.h
index fd3eb16f5..724c68608 100644
--- a/gtsam/inference/EliminationTree.h
+++ b/gtsam/inference/EliminationTree.h
@@ -19,7 +19,6 @@
 
 #include <utility>
 #include <memory>
-#include <queue>
 
 #include <gtsam/base/Testable.h>
 #include <gtsam/base/FastVector.h>
@@ -119,24 +118,7 @@ namespace gtsam {
     /// @}
 
   public:
-    ~EliminationTree() {
-      // use default destructor which recursively deletes all nodes with shared_ptr causes stack overflow.
-      // so for each tree, we first move the root into a queue; then we do a BFS on the tree with the queue;
-      // for each node iterated, if its reference count is 1, it will be deleted while its children are still in the queue.
-      // so that the recursive deletion will not happen.
-      for (auto&& root : roots_) {
-        std::queue<sharedNode> bfs_queue;
-        bfs_queue.push(root);
-        root = nullptr;
-        while (!bfs_queue.empty()) {
-          auto node = bfs_queue.front();
-          bfs_queue.pop();
-          for (auto&& child : node->children) {
-            bfs_queue.push(child);
-          }
-        }
-      }
-    }
+    ~EliminationTree(); 
     /// @name Standard Interface
     /// @{