From 00a20409f716e187cbe85a20310f7a9444ba67bd Mon Sep 17 00:00:00 2001 From: Thomas Geymayer Date: Tue, 29 Sep 2015 21:38:52 +0200 Subject: [PATCH] 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. --- simgear/canvas/elements/CanvasElement.cxx | 35 ++++++++--------------- simgear/canvas/elements/CanvasElement.hxx | 6 ++-- simgear/canvas/elements/CanvasGroup.cxx | 2 +- simgear/canvas/elements/CanvasGroup.hxx | 2 +- simgear/canvas/elements/CanvasImage.cxx | 2 +- simgear/canvas/elements/CanvasImage.hxx | 2 +- simgear/canvas/elements/CanvasMap.cxx | 2 +- simgear/canvas/elements/CanvasMap.hxx | 2 +- simgear/canvas/elements/CanvasPath.cxx | 2 +- simgear/canvas/elements/CanvasPath.hxx | 2 +- simgear/canvas/elements/CanvasText.cxx | 2 +- simgear/canvas/elements/CanvasText.hxx | 2 +- 12 files changed, 25 insertions(+), 36 deletions(-) diff --git a/simgear/canvas/elements/CanvasElement.cxx b/simgear/canvas/elements/CanvasElement.cxx index 6d8c9305..7ae32494 100644 --- 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(_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 37e12dea..09b6b421 100644 --- 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 e2c88885..de289a8d 100644 --- 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 8825d909..6a047f0b 100644 --- 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 4dc0b341..6c037d70 100644 --- 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 73e6d4d5..2ac255cf 100644 --- 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 80710f50..d05c1ec7 100644 --- 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 4af74ac7..993d45da 100644 --- 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 a2d0c1a5..4ad35734 100644 --- 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 9cd3aef5..a1d4a8e1 100644 --- 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 0d108b1b..06ad5dff 100644 --- 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 27754dd6..a04ba4e5 100644 --- 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);