Fixed|Client|World: Memory leak in the BSP builder

LineSegmentBlock objects were not being deleted.
skyjake · Jan 22, 2017 · 4e7c7da · 4e7c7da
1 parent 9d06a5d
commit 4e7c7da
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 15 deletions.
diff --git a/doomsday/apps/client/include/world/bsp/superblockmap.h b/doomsday/apps/client/include/world/bsp/superblockmap.h
@@ -86,7 +86,17 @@ class LineSegmentBlock
     DENG2_PRIVATE(d)
 };
 
-typedef de::BinaryTree<LineSegmentBlock *> LineSegmentBlockTreeNode;
+struct LineSegmentBlockTreeNode : public de::BinaryTree<LineSegmentBlock *>
+{
+    LineSegmentBlockTreeNode(LineSegmentBlock *lsb, // owned
+                             LineSegmentBlockTreeNode *parent = nullptr);
+    ~LineSegmentBlockTreeNode();
+
+    LineSegmentBlockTreeNode *childPtr(ChildId side) const;
+    LineSegmentBlockTreeNode *parentPtr() const;
+    LineSegmentBlockTreeNode *rightPtr() const { return childPtr(Right); }
+    LineSegmentBlockTreeNode *leftPtr() const  { return childPtr(Left); }
+};
 
 }  // namespace bsp
 }  // namespace world

diff --git a/doomsday/apps/client/src/world/bsp/partitioner.cpp b/doomsday/apps/client/src/world/bsp/partitioner.cpp
@@ -82,26 +82,22 @@ DENG2_PIMPL(Partitioner)
             : rootNode(new LineSegmentBlockTreeNode(new LineSegmentBlock(bounds)))
         {}
 
-        ~LineSegmentBlockTree() { clear(); }
+        ~LineSegmentBlockTree()
+        {
+            clear();
+        }
 
         /// Implicit conversion from LineSegmentBlockTree to root tree node.
-        inline operator LineSegmentBlockTreeNode /*const*/ &() {
+        inline operator LineSegmentBlockTreeNode /*const*/ &()
+        {
             return *rootNode;
         }
 
     private:
-        static int clearUserDataWorker(LineSegmentBlockTreeNode &subtree, void *)
-        {
-            delete subtree.userData();
-            subtree.setUserData(nullptr);
-            return 0; // Continue iteration.
-        }
-
         void clear()
         {
-            if(!rootNode) return;
-            rootNode->traversePostOrder(clearUserDataWorker);
-            delete rootNode; rootNode = nullptr;
+            delete rootNode;
+            rootNode = nullptr;
         }
     };
 

diff --git a/doomsday/apps/client/src/world/bsp/partitionevaluator.cpp b/doomsday/apps/client/src/world/bsp/partitionevaluator.cpp
@@ -370,11 +370,11 @@ DENG2_PIMPL_NOREF(PartitionEvaluator)
 
             if(node.hasRight())
             {
-                costForBlock(node.right());
+                costForBlock(*node.rightPtr());
             }
             if(node.hasLeft())
             {
-                costForBlock(node.left());
+                costForBlock(*node.leftPtr());
             }
         }
     };

diff --git a/doomsday/apps/client/src/world/bsp/superblockmap.cpp b/doomsday/apps/client/src/world/bsp/superblockmap.cpp
@@ -95,5 +95,29 @@ LineSegmentBlock::All const &LineSegmentBlock::all() const
     return d->segments;
 }
 
+//---------------------------------------------------------------------------------------
+
+LineSegmentBlockTreeNode::LineSegmentBlockTreeNode(LineSegmentBlock *lsb,
+                                                   LineSegmentBlockTreeNode *parent)
+    : BinaryTree<LineSegmentBlock *>(lsb, parent)
+{}
+
+LineSegmentBlockTreeNode::~LineSegmentBlockTreeNode()
+{
+    delete userData();
+}
+
+LineSegmentBlockTreeNode *LineSegmentBlockTreeNode::childPtr(ChildId side) const
+{
+    return static_cast<LineSegmentBlockTreeNode *>
+            (BinaryTree<LineSegmentBlock *>::childPtr(side));
+}
+
+LineSegmentBlockTreeNode *LineSegmentBlockTreeNode::parentPtr() const
+{
+    return static_cast<LineSegmentBlockTreeNode *>
+            (BinaryTree<LineSegmentBlock *>::parentPtr());
+}
+
 }  // namespace bsp
 }  // namespace world