[Orxonox-commit 3217] r7910 - code/trunk/src/orxonox/worldentities

[landauf at orxonox.net]

Author: landauf
Date: 2011-02-18 00:07:04 +0100 (Fri, 18 Feb 2011)
New Revision: 7910

Modified:
   code/trunk/src/orxonox/worldentities/WorldEntity.cc
   code/trunk/src/orxonox/worldentities/WorldEntity.h
Log:
Ok, here comes a tricky one: we knew the destructor of WorldEntity is dangerous because it deletes its children - this works only if no child deletes the subsequent child in the set because then the iterator would become invalid. However this is never the case, right?

Wrong! If you define a CameraPosition with "absolute" flag, it will not be attached to a ControllableEntity but to its parent! The CE however still deletes the CameraPosition. This led to very rare and random crashes while destroying the Pong level (the only level where an absolute camera is used).

 - fixed this issue by using a safe destruction loop
 - fixed another issue in the same loop if a child has a smart pointer pointing to it, thus not being destroyed, hence it has to be detached
 - performance improvement in detach()

Thanks to Jo for sending me the crash log!


Modified: code/trunk/src/orxonox/worldentities/WorldEntity.cc
===================================================================
--- code/trunk/src/orxonox/worldentities/WorldEntity.cc	2011-02-17 16:15:53 UTC (rev 7909)
+++ code/trunk/src/orxonox/worldentities/WorldEntity.cc	2011-02-17 23:07:04 UTC (rev 7910)
@@ -119,15 +119,15 @@
             if (this->parent_)
                 this->detachFromParent();
 
-            for (std::set<WorldEntity*>::const_iterator it = this->children_.begin(); it != this->children_.end(); )
+            std::set<WorldEntity*>::iterator it;
+            while ((it = this->children_.begin()) != this->children_.end())
             {
+                // do this for all children, because even if getDeleteWithParent() returns true a child might still stay active due to smart pointers pointing to it
+                (*it)->setPosition((*it)->getWorldPosition());
+                this->detach(*it); // detach also erases the element from the children set
+
                 if ((*it)->getDeleteWithParent())
-                    (*(it++))->destroy();
-                else
-                {
-                    (*it)->setPosition((*it)->getWorldPosition());
-                    this->detach(*(it++));
-                }
+                    (*it)->destroy();
             }
 
             if (this->physicalBody_)
@@ -449,7 +449,8 @@
     */
     void WorldEntity::detach(WorldEntity* object)
     {
-        if (this->children_.find(object) == this->children_.end())
+        std::set<WorldEntity*>::iterator it = this->children_.find(object);
+        if (it == this->children_.end())
         {
             CCOUT(2) << "Warning: Cannot detach an object that is not a child." << std::endl;
             return;
@@ -466,7 +467,7 @@
         }
 
         this->detachNode(object->node_);
-        this->children_.erase(object);
+        this->children_.erase(it);
 
         object->notifyDetached();
     }

Modified: code/trunk/src/orxonox/worldentities/WorldEntity.h
===================================================================
--- code/trunk/src/orxonox/worldentities/WorldEntity.h	2011-02-17 16:15:53 UTC (rev 7909)
+++ code/trunk/src/orxonox/worldentities/WorldEntity.h	2011-02-17 23:07:04 UTC (rev 7910)
@@ -213,7 +213,7 @@
 
         private:
             void registerVariables();
-            
+
             inline void lookAt_xmlport(const Vector3& target)
                 { this->lookAt(target); }
             inline void setDirection_xmlport(const Vector3& direction)