Revert recursive listeners for the moment.

Want to verify this is the cause of crashes inside the property code,
since if it's not we have a much bigger problem.

This means all listeners are recursive, with a parent-chain walk on each
setValue call, as has been the case since 'forever'.

simgear/props/PropertyBasedElement.cxx

@@ -24,7 +24,6 @@ namespace simgear
 
   //----------------------------------------------------------------------------
   PropertyBasedElement::PropertyBasedElement(SGPropertyNode* node):
-    SGPropertyChangeListener(true /* recursive */),
     _node(node)
   {
     _node->addChangeListener(this);

simgear/props/PropertyBasedMgr.cxx

@@ -30,6 +30,7 @@ namespace simgear
   void PropertyBasedMgr::init()
   {
     _props->addChangeListener(this);
+    _props->fireCreatedRecursive();
   }
 
   //----------------------------------------------------------------------------
@@ -91,7 +92,6 @@ namespace simgear
   PropertyBasedMgr::PropertyBasedMgr( SGPropertyNode_ptr props,
                                       const std::string& name_elements,
                                       ElementFactory element_factory ):
-    SGPropertyChangeListener(true /* recursive*/),
     _props( props ),
     _name_elements( name_elements ),
     _element_factory( element_factory )

simgear/props/props.cxx

@@ -34,6 +34,7 @@ using std::cerr;
 # include <boost/range.hpp>
 # include <simgear/compiler.h>
 # include <simgear/debug/logstream.hxx>
+# include <simgear/sg_inlines.h>
 
 # include "PropertyInterpolationMgr.hxx"
 # include "vectorPropTemplates.hxx"
@@ -48,8 +49,6 @@ using std::stringstream;
 
 using namespace simgear;
 
-#define ALTERNATE_RECURSIVE 1
-
 ////////////////////////////////////////////////////////////////////////
 // Local classes.
 ////////////////////////////////////////////////////////////////////////
@@ -2361,34 +2360,10 @@ SGPropertyNode::addChangeListener (SGPropertyChangeListener * listener,
 {
   if (_listeners == 0)
     _listeners = new vector<SGPropertyChangeListener*>;
-
+  _listeners->push_back(listener);
-    auto it = std::find(_listeners->begin(), _listeners->end(), listener);
+  listener->register_property(this);
-    if (it != _listeners->end()) {
+  if (initial)
-        return;
+    listener->valueChanged(this);
-    }
-
-    _listeners->push_back(listener);
-    listener->register_property(this);
-
-#ifdef ALTERNATE_RECURSIVE
-    if (listener->isRecursive()) {
-        for (auto child : _children) {
-            if (initial) {
-                fireChildAdded(child);
-            }
-            child->addChangeListener(listener, initial);
-        }
-    }
-
-    if (initial) {
-        listener->doValueChanged(this);
-    }
-#else
-    if (initial) {
-        listener->doValueChanged(this);
-        fireCreatedRecursive(false);
-    }
-#endif
 }
 
 void
@@ -2396,7 +2371,6 @@ SGPropertyNode::removeChangeListener (SGPropertyChangeListener * listener)
 {
   if (_listeners == 0)
     return;
-
   vector<SGPropertyChangeListener*>::iterator it =
     find(_listeners->begin(), _listeners->end(), listener);
   if (it != _listeners->end()) {
@@ -2407,14 +2381,6 @@ SGPropertyNode::removeChangeListener (SGPropertyChangeListener * listener)
       _listeners = 0;
       delete tmp;
     }
-
-#ifdef ALTERNATE_RECURSIVE
-      if (listener->isRecursive()) {
-          for (auto child : _children) {
-              child->removeChangeListener(listener);
-          }
-      }
-#endif
   }
 }
 
@@ -2467,13 +2433,11 @@ SGPropertyNode::fireValueChanged (SGPropertyNode * node)
 {
   if (_listeners != 0) {
     for (unsigned int i = 0; i < _listeners->size(); i++) {
-      (*_listeners)[i]->doValueChanged(node);
+      (*_listeners)[i]->valueChanged(node);
     }
   }
-#if !defined(ALTERNATE_RECURSIVE)
   if (_parent != 0)
     _parent->fireValueChanged(node);
-#endif
 }
 
 void
@@ -2481,19 +2445,12 @@ SGPropertyNode::fireChildAdded (SGPropertyNode * parent,
 				SGPropertyNode * child)
 {
   if (_listeners != 0) {
-      for (auto listen : *_listeners) {
+    for (unsigned int i = 0; i < _listeners->size(); i++) {
-          listen->doChildAdded(parent, child);
+      (*_listeners)[i]->childAdded(parent, child);
-#if defined(ALTERNATE_RECURSIVE)
-          if (listen->isRecursive()) {
-              child->addChangeListener(listen);
-          }
-#endif
     }
   }
-#if !defined(ALTERNATE_RECURSIVE)
   if (_parent != 0)
     _parent->fireChildAdded(parent, child);
-#endif
 }
 
 void
@@ -2501,19 +2458,12 @@ SGPropertyNode::fireChildRemoved (SGPropertyNode * parent,
 				  SGPropertyNode * child)
 {
   if (_listeners != 0) {
-    for (auto listen : *_listeners) {
+    for (unsigned int i = 0; i < _listeners->size(); i++) {
-        listen->childRemoved(parent, child);
+      (*_listeners)[i]->childRemoved(parent, child);
-#if defined(ALTERNATE_RECURSIVE)
-        if (listen) {
-            child->removeChangeListener(listen);
-        }
-#endif
     }
   }
-#if !defined(ALTERNATE_RECURSIVE)
   if (_parent != 0)
     _parent->fireChildRemoved(parent, child);
-#endif
 }
 
 //------------------------------------------------------------------------------
@@ -2533,9 +2483,9 @@ SGPropertyNode::eraseChild(simgear::PropertyList::iterator child)
 // Implementation of SGPropertyChangeListener.
 ////////////////////////////////////////////////////////////////////////
 
-SGPropertyChangeListener::SGPropertyChangeListener(bool recursive) :
+SGPropertyChangeListener::SGPropertyChangeListener(bool recursive)
-    _recursive(recursive)
 {
+    SG_UNUSED(recursive); // for the moment, all listeners are recursive
 }
 
 SGPropertyChangeListener::~SGPropertyChangeListener ()
@@ -2554,16 +2504,7 @@ void
 SGPropertyChangeListener::childAdded (SGPropertyNode * node,
 				      SGPropertyNode * child)
 {
-}
+  // NO-OP
-
-void SGPropertyChangeListener::doChildAdded(SGPropertyNode * parent, SGPropertyNode * child)
-{
-    childAdded(parent, child);
-}
-
-void SGPropertyChangeListener::doValueChanged(SGPropertyNode * node)
-{
-    valueChanged(node);
 }
 
 void
@@ -2576,26 +2517,15 @@ SGPropertyChangeListener::childRemoved (SGPropertyNode * parent,
 void
 SGPropertyChangeListener::register_property (SGPropertyNode * node)
 {
-    auto it = std::lower_bound(_properties.begin(), _properties.end(), node);
+  _properties.push_back(node);
-    if (it != _properties.end()) {
-        if (*it == node) {
-            // already exists, duplicate listent
-            return;
-        }
-    }
-
-    _properties.insert(it, node);
 }
 
 void
 SGPropertyChangeListener::unregister_property (SGPropertyNode * node)
 {
-    auto it = std::lower_bound(_properties.begin(), _properties.end(), node);
+  vector<SGPropertyNode *>::iterator it =
-    if (it == _properties.end()) {
+    find(_properties.begin(), _properties.end(), node);
-        // not found, warn?
+  if (it != _properties.end())
-        return;
-    }
-
     _properties.erase(it);
 }
 

simgear/props/props.hxx

@@ -22,8 +22,6 @@
 #include <sstream>
 #include <typeinfo>
 
-#include <cmath> // for fabs
-
 #include <simgear/compiler.h>
 #if PROPS_STANDALONE
 // taken from: boost/utility/enable_if.hpp
@@ -238,12 +236,19 @@ public:
      */
     virtual SGRaw* clone() const = 0;
 
-
 };
 
 class SGRawExtended : public SGRaw
 {
 public:
+    /**    
+     * Make an SGRawValueContainer from the SGRawValue.
+     *
+     * This is a virtual function of SGRawExtended so that
+     * SGPropertyNode::untie doesn't need to know the type of an
+     * extended property.
+     */
+    virtual SGRawExtended* makeContainer() const = 0;
     /**
      * Write value out to a stream
      */
@@ -252,15 +257,6 @@ public:
      * Read value from a stream and store it.
      */
     virtual std::istream& readFrom(std::istream& stream) = 0;
-
-    /**
-     * Make an SGRawValueContainer from the SGRawValue.
-     *
-     * This is a virtual function of SGRawExtended so that
-     * SGPropertyNode::untie doesn't need to know the type of an
-     * extended property.
-     */
-    virtual SGRawExtended* makeContainer() const = 0;
 };
 
 // Choose between different base classes based on whether the value is
@@ -399,7 +395,6 @@ template<> inline const char * SGRawValue<const char *>::DefaultValue()
   return "";
 }
 
-
 /**
  * A raw value bound to a pointer.
  *
@@ -688,7 +683,8 @@ private:
 template<typename T>
 SGRawExtended* SGRawBase<T, 0>::makeContainer() const
 {
-    return new SGRawValueContainer<T>(static_cast<const SGRawValue<T>*>(this)->getValue());
+    return new SGRawValueContainer<T>(static_cast<const SGRawValue<T>*>(this)
+                                      ->getValue());
 }
 
 template<typename T>
@@ -738,22 +734,14 @@ public:
   /// Called if \a child has been removed from its \a parent.
   virtual void childRemoved(SGPropertyNode * parent, SGPropertyNode * child);
 
-    bool isRecursive() const { return _recursive; }
-
 protected:
     SGPropertyChangeListener(bool recursive = false);
-
+  friend class SGPropertyNode;
-    friend class SGPropertyNode;
+  virtual void register_property (SGPropertyNode * node);
-
+  virtual void unregister_property (SGPropertyNode * node);
-    virtual void register_property (SGPropertyNode * node);
-    virtual void unregister_property (SGPropertyNode * node);
-
-    void doValueChanged(SGPropertyNode * node);
-    void doChildAdded(SGPropertyNode * parent, SGPropertyNode * child);
 
 private:
-    const bool _recursive = false;
+  std::vector<SGPropertyNode *> _properties;
-    std::vector<SGPropertyNode *> _properties;
 };
 
 
@@ -1155,6 +1143,7 @@ public:
     (state ? _attr |= attr : _attr &= ~attr);
   }
 
+
   /**
    * Get all of the mode attributes for the property node.
    */
@@ -1773,8 +1762,6 @@ protected:
   static simgear::PropertyInterpolationMgr* _interpolation_mgr;
 
 private:
-    // friend so we can poll the SGRaw for tied properties directly.
-    friend class SGPropertyChangeListener;
 
   // Get the raw value
   bool get_bool () const;

simgear/props/props_test.cxx

@@ -433,7 +433,6 @@ class TestListener : public SGPropertyChangeListener
 {
 public:
     TestListener(SGPropertyNode* root, bool recursive = false) :
-        SGPropertyChangeListener(recursive),
         _root(root) {}
 
     virtual void valueChanged(SGPropertyNode* node) override
@@ -612,6 +611,8 @@ void testListener()
     defineSamplePropertyTree(tree);
 
     // ensure basic listenter is not recursive
+    // disabled for now, since all listeners are recurisve
+#if 0
     {
         TestListener l(tree.get(), false /* not recursive */);
         tree->getNode("position/body")->addChangeListener(&l);
@@ -629,6 +630,7 @@ void testListener()
         SG_VERIFY(l.checkAdded("position/body",
                                tree->getNode("position/body/new")));
     }
+#endif
 
     tree = new SGPropertyNode;
     defineSamplePropertyTree(tree);
@@ -702,11 +704,13 @@ void testListener()
 
         // ensure additional calls to fireChildrenRecursive don't cause multiple adds
 
-        SG_VERIFY(ensureNListeners(tree->getNode("position/body"), 1));
+        // disabled for now since we don't add recursive listeners to sub-trees
+      //  SG_VERIFY(ensureNListeners(tree->getNode("position/body"), 1));
         SG_VERIFY(ensureNListeners(tree->getNode("controls"), 0));
 
         tree->getNode("position/body")->fireCreatedRecursive();
-        SG_VERIFY(ensureNListeners(tree->getNode("position/body"), 1));
+        // disabled for now since we don't add recursive listeners to sub-trees
+        //SG_VERIFY(ensureNListeners(tree->getNode("position/body"), 1));
     }
 
 }