Canvas: use weak pointer to protect parent element access.

Using a weak pointer is the best way to ensure no invalid
pointer is used. This also fixes a possible crash in
simgear::canvas::Element::getParentStyle on destructing
canvas elements.

diff --git a/simgear/canvas/elements/CanvasElement.cxx b/simgear/canvas/elements/CanvasElement.cxx
index 6d8c93058e95f8a7cb4b34c26c91e0eabb2e12de..7ae324947cc2d3532579aa3b07794576e2452b5b 100644 (file)
--- a/simgear/canvas/elements/CanvasElement.cxx
+++ b/simgear/canvas/elements/CanvasElement.cxx
@@ -207,19 +207,6 @@ namespace canvas
   //----------------------------------------------------------------------------
   Element::~Element()
   {
-    if( !_transform.valid() )
-      return;
-
-    for(unsigned int i = 0; i < _transform->getNumChildren(); ++i)
-    {
-      OSGUserData* ud =
-        static_cast<OSGUserData*>(_transform->getChild(i)->getUserData());
-
-      if( ud )
-        // Ensure parent is cleared to prevent accessing released memory if an
-        // element somehow survives longer than his parent.
-        ud->element->_parent = 0;
-    }
   }
 
   //----------------------------------------------------------------------------
@@ -246,7 +233,7 @@ namespace canvas
   //----------------------------------------------------------------------------
   ElementPtr Element::getParent() const
   {
-    return _parent;
+    return _parent.lock();
   }
 
   //----------------------------------------------------------------------------
@@ -324,8 +311,9 @@ namespace canvas
   //----------------------------------------------------------------------------
   bool Element::ascend(EventVisitor& visitor)
   {
-    if( _parent )
-      return _parent->accept(visitor);
+    ElementPtr parent = getParent();
+    if( parent )
+      return parent->accept(visitor);
     return true;
   }
 
@@ -374,9 +362,9 @@ namespace canvas
     EventPropagationPath path;
     path.push_back( EventTarget(this) );
 
-    for( Element* parent = _parent;
-                  parent != NULL;
-                  parent = parent->_parent )
+    for( ElementPtr parent = getParent();
+                    parent.valid();
+                    parent = parent->getParent() )
       path.push_front( EventTarget(parent) );
 
     CanvasPtr canvas = _canvas.lock();
@@ -772,7 +760,7 @@ namespace canvas
   Element::Element( const CanvasWeakPtr& canvas,
                     const SGPropertyNode_ptr& node,
                     const Style& parent_style,
-                    Element* parent ):
+                    ElementWeakPtr parent ):
     PropertyBasedElement(node),
     _canvas( canvas ),
     _parent( parent ),
@@ -871,11 +859,12 @@ namespace canvas
   Element::getParentStyle(const SGPropertyNode* child) const
   {
     // Try to get value from parent...
-    if( _parent )
+    ElementPtr parent = getParent();
+    if( parent )
     {
       Style::const_iterator style =
-        _parent->_style.find(child->getNameString());
-      if( style != _parent->_style.end() )
+        parent->_style.find(child->getNameString());
+      if( style != parent->_style.end() )
         return style->second;
     }
 

diff --git a/simgear/canvas/elements/CanvasElement.hxx b/simgear/canvas/elements/CanvasElement.hxx
index 37e12deac95e271c7b611fc4675c906d140de618..09b6b42116b082ff6c7e80d8ac12da10a227d0bd 100644 (file)
--- a/simgear/canvas/elements/CanvasElement.hxx
+++ b/simgear/canvas/elements/CanvasElement.hxx
@@ -232,8 +232,8 @@ namespace canvas
 
       class RelativeScissor;
 
-      CanvasWeakPtr _canvas;
-      Element      *_parent;
+      CanvasWeakPtr   _canvas;
+      ElementWeakPtr  _parent;
 
       mutable uint32_t _attributes_dirty;
 
@@ -256,7 +256,7 @@ namespace canvas
       Element( const CanvasWeakPtr& canvas,
                const SGPropertyNode_ptr& node,
                const Style& parent_style,
-               Element* parent );
+               ElementWeakPtr parent );
 
       /**
        * Returns false on first call and true on any successive call. Use to

diff --git a/simgear/canvas/elements/CanvasGroup.cxx b/simgear/canvas/elements/CanvasGroup.cxx
index e2c888855b6bab2171e3074b77dcab642b270f36..de289a8d0553aef5121b2b20cefc7163e22d42f6 100644 (file)
--- a/simgear/canvas/elements/CanvasGroup.cxx
+++ b/simgear/canvas/elements/CanvasGroup.cxx
@@ -70,7 +70,7 @@ namespace canvas
   Group::Group( const CanvasWeakPtr& canvas,
                 const SGPropertyNode_ptr& node,
                 const Style& parent_style,
-                Element* parent ):
+                ElementWeakPtr parent ):
     Element(canvas, node, parent_style, parent)
   {
     staticInit();

diff --git a/simgear/canvas/elements/CanvasGroup.hxx b/simgear/canvas/elements/CanvasGroup.hxx
index 8825d90940dc8060b00dc88115e2fa956f90da13..6a047f0be8eaef472362db9dd14e6e0ab4df38a3 100644 (file)
--- a/simgear/canvas/elements/CanvasGroup.hxx
+++ b/simgear/canvas/elements/CanvasGroup.hxx
@@ -45,7 +45,7 @@ namespace canvas
       Group( const CanvasWeakPtr& canvas,
              const SGPropertyNode_ptr& node,
              const Style& parent_style = Style(),
-             Element* parent = 0 );
+             ElementWeakPtr parent = 0 );
       virtual ~Group();
 
       ElementPtr createChild( const std::string& type,

diff --git a/simgear/canvas/elements/CanvasImage.cxx b/simgear/canvas/elements/CanvasImage.cxx
index 4dc0b34129ce06d0d63cb60058e29f607c5397e5..6c037d703811bfe8d2248bb21784942b42cf4bef 100644 (file)
--- a/simgear/canvas/elements/CanvasImage.cxx
+++ b/simgear/canvas/elements/CanvasImage.cxx
@@ -112,7 +112,7 @@ namespace canvas
   Image::Image( const CanvasWeakPtr& canvas,
                 const SGPropertyNode_ptr& node,
                 const Style& parent_style,
-                Element* parent ):
+                ElementWeakPtr parent ):
     Element(canvas, node, parent_style, parent),
     _texture(new osg::Texture2D),
     _node_src_rect( node->getNode("source", 0, true) ),

diff --git a/simgear/canvas/elements/CanvasImage.hxx b/simgear/canvas/elements/CanvasImage.hxx
index 73e6d4d55a1e5baf6cfe431a9b252ef79f5486f7..2ac255cfc13eea22663d42d3e4ec5efe5859d57f 100644 (file)
--- a/simgear/canvas/elements/CanvasImage.hxx
+++ b/simgear/canvas/elements/CanvasImage.hxx
@@ -50,7 +50,7 @@ namespace canvas
       Image( const CanvasWeakPtr& canvas,
              const SGPropertyNode_ptr& node,
              const Style& parent_style = Style(),
-             Element* parent = 0 );
+             ElementWeakPtr parent = 0 );
       virtual ~Image();
 
       virtual void update(double dt);

diff --git a/simgear/canvas/elements/CanvasMap.cxx b/simgear/canvas/elements/CanvasMap.cxx
index 80710f50b2797c2283e369265d22901120099e47..d05c1ec77ef119b066242eb25f688ea0da643788 100644 (file)
--- a/simgear/canvas/elements/CanvasMap.cxx
+++ b/simgear/canvas/elements/CanvasMap.cxx
@@ -62,7 +62,7 @@ namespace canvas
   Map::Map( const CanvasWeakPtr& canvas,
             const SGPropertyNode_ptr& node,
             const Style& parent_style,
-            Element* parent ):
+            ElementWeakPtr parent ):
     Group(canvas, node, parent_style, parent),
     // TODO make projection configurable
     _projection(new SansonFlamsteedProjection),

diff --git a/simgear/canvas/elements/CanvasMap.hxx b/simgear/canvas/elements/CanvasMap.hxx
index 4af74ac77ff1af5b12b03a9d99facbeeab15a09b..993d45daef79f0af90f43006898b6b201a6e4e65 100644 (file)
--- a/simgear/canvas/elements/CanvasMap.hxx
+++ b/simgear/canvas/elements/CanvasMap.hxx
@@ -42,7 +42,7 @@ namespace canvas
       Map( const CanvasWeakPtr& canvas,
            const SGPropertyNode_ptr& node,
            const Style& parent_style,
-           Element* parent = 0 );
+           ElementWeakPtr parent = 0 );
       virtual ~Map();
 
       virtual void update(double dt);

diff --git a/simgear/canvas/elements/CanvasPath.cxx b/simgear/canvas/elements/CanvasPath.cxx
index a2d0c1a57be24c3b7a4b90485b4f8b5466e0e03a..4ad357349f9c833659f80d18880c621986229bd5 100644 (file)
--- a/simgear/canvas/elements/CanvasPath.cxx
+++ b/simgear/canvas/elements/CanvasPath.cxx
@@ -531,7 +531,7 @@ namespace canvas
   Path::Path( const CanvasWeakPtr& canvas,
               const SGPropertyNode_ptr& node,
               const Style& parent_style,
-              Element* parent ):
+              ElementWeakPtr parent ):
     Element(canvas, node, parent_style, parent),
     _path( new PathDrawable(this) )
   {

diff --git a/simgear/canvas/elements/CanvasPath.hxx b/simgear/canvas/elements/CanvasPath.hxx
index 9cd3aef589e48385d4e53c7c3bf8dd16ad5a5f14..a1d4a8e12b553f1ce75ec3e06682e1185dc7a2f7 100644 (file)
--- a/simgear/canvas/elements/CanvasPath.hxx
+++ b/simgear/canvas/elements/CanvasPath.hxx
@@ -36,7 +36,7 @@ namespace canvas
       Path( const CanvasWeakPtr& canvas,
             const SGPropertyNode_ptr& node,
             const Style& parent_style,
-            Element* parent = 0 );
+            ElementWeakPtr parent = 0 );
       virtual ~Path();
 
       virtual void update(double dt);

diff --git a/simgear/canvas/elements/CanvasText.cxx b/simgear/canvas/elements/CanvasText.cxx
index 0d108b1b59f84e6fa4cf6f565eac62f8a5ef4d07..06ad5dff149d21f4226868d7c562ed9dfc0c97b5 100644 (file)
--- a/simgear/canvas/elements/CanvasText.cxx
+++ b/simgear/canvas/elements/CanvasText.cxx
@@ -740,7 +740,7 @@ namespace canvas
   Text::Text( const CanvasWeakPtr& canvas,
               const SGPropertyNode_ptr& node,
               const Style& parent_style,
-              Element* parent ):
+              ElementWeakPtr parent ):
     Element(canvas, node, parent_style, parent),
     _text( new Text::TextOSG(this) )
   {

diff --git a/simgear/canvas/elements/CanvasText.hxx b/simgear/canvas/elements/CanvasText.hxx
index 27754dd6047668674850ae43e0fa02db00411755..a04ba4e5a229e48be406bfd765461a75027a3c57 100644 (file)
--- a/simgear/canvas/elements/CanvasText.hxx
+++ b/simgear/canvas/elements/CanvasText.hxx
@@ -40,7 +40,7 @@ namespace canvas
       Text( const CanvasWeakPtr& canvas,
             const SGPropertyNode_ptr& node,
             const Style& parent_style,
-            Element* parent = 0 );
+            ElementWeakPtr parent = 0 );
       ~Text();
 
       void setText(const char* text);