CO hash join and memory leak fix

src/control/control.cpp

@@ -7,9 +7,10 @@
 #include "util/timer.h"
 
 // Static member variable definition
-QHash<ConfigKey, ControlDoublePrivate*> ControlDoublePrivate::m_sqCOHash;
+QHash<ConfigKey, QWeakPointer<ControlDoublePrivate> > ControlDoublePrivate::m_sqCOHash;
 QMutex ControlDoublePrivate::m_sqCOHashMutex;
 
+/*
 ControlDoublePrivate::ControlDoublePrivate()
         : m_bIgnoreNops(true),
           m_bTrack(false),
@@ -19,8 +20,9 @@ ControlDoublePrivate::ControlDoublePrivate()
           m_confirmRequired(false) {
     initialize();
 }
+*/
 
-ControlDoublePrivate::ControlDoublePrivate(ConfigKey key,
+ControlDoublePrivate::ControlDoublePrivate(ConfigKey key, ControlObject* pCreatorCO,
                                            bool bIgnoreNops, bool bTrack)
         : m_key(key),
           m_bIgnoreNops(bIgnoreNops),
@@ -29,18 +31,15 @@ ControlDoublePrivate::ControlDoublePrivate(ConfigKey key,
           m_trackType(Stat::UNSPECIFIED),
           m_trackFlags(Stat::COUNT | Stat::SUM | Stat::AVERAGE |
                        Stat::SAMPLE_VARIANCE | Stat::MIN | Stat::MAX),
-          m_confirmRequired(false) {
+          m_confirmRequired(false),
+          m_pCreatorCO(pCreatorCO) {
     initialize();
 }
 
 void ControlDoublePrivate::initialize() {
     m_defaultValue.setValue(0);
     m_value.setValue(0);
 
-    m_sqCOHashMutex.lock();
-    m_sqCOHash.insert(m_key, this);
-    m_sqCOHashMutex.unlock();
-
     if (m_bTrack) {
         // TODO(rryan): Make configurable.
         Stat::track(m_trackKey, static_cast<Stat::StatType>(m_trackType),
@@ -51,36 +50,52 @@ void ControlDoublePrivate::initialize() {
 
 ControlDoublePrivate::~ControlDoublePrivate() {
     m_sqCOHashMutex.lock();
+    //qDebug() << "ControlDoublePrivate::m_sqCOHash.remove(" << m_key.group << "," << m_key.item << ")";
     m_sqCOHash.remove(m_key);
     m_sqCOHashMutex.unlock();
 }
 
 // static
-ControlDoublePrivate* ControlDoublePrivate::getControl(
-        const ConfigKey& key, bool bCreate, bool bIgnoreNops, bool bTrack) {
+QSharedPointer<ControlDoublePrivate> ControlDoublePrivate::getControl(
+        const ConfigKey& key, ControlObject* pCreatorCO, bool bIgnoreNops, bool bTrack) {
     QMutexLocker locker(&m_sqCOHashMutex);
-    QHash<ConfigKey, ControlDoublePrivate*>::const_iterator it = m_sqCOHash.find(key);
+    QSharedPointer<ControlDoublePrivate> pControl;
+    QHash<ConfigKey, QWeakPointer<ControlDoublePrivate> >::const_iterator it = m_sqCOHash.find(key);
     if (it != m_sqCOHash.end()) {
-        return it.value();
+        if (pCreatorCO) {
+            qDebug() << "ControlObject" << key.group << key.item << "already created";
+        } else {
+            pControl = it.value();
+        }
     }
     locker.unlock();
 
-    ControlDoublePrivate* pControl = NULL;
-    if (bCreate) {
-        pControl = new ControlDoublePrivate(key, bIgnoreNops, bTrack);
-        locker.relock();
-        m_sqCOHash.insert(key, pControl);
-        locker.unlock();
-    }
-
     if (pControl == NULL) {
-        qWarning() << "ControlDoublePrivate::getControl returning NULL for ("
-                   << key.group << "," << key.item << ")";
+        if (pCreatorCO) {
+            pControl = QSharedPointer<ControlDoublePrivate>(
+                    new ControlDoublePrivate(key, pCreatorCO, bIgnoreNops, bTrack));
+            locker.relock();
+            //qDebug() << "ControlDoublePrivate::m_sqCOHash.insert(" << key.group << "," << key.item << ")";
+            m_sqCOHash.insert(key, pControl);
+            locker.unlock();
+        } else {
+            qWarning() << "ControlDoublePrivate::getControl returning NULL for ("
+                       << key.group << "," << key.item << ")";
+        }
     }
-
     return pControl;
 }
 
+// static
+void ControlDoublePrivate::getControls(QList<ControlDoublePrivate*>* pControlList) {
+    m_sqCOHashMutex.lock();
+    for (QHash<ConfigKey, QWeakPointer<ControlDoublePrivate> >::const_iterator it = m_sqCOHash.begin();
+         it != m_sqCOHash.end(); ++it) {
+        pControlList->push_back(it.value().data());
+    }
+    m_sqCOHashMutex.unlock();
+}
+
 double ControlDoublePrivate::get() const {
     return m_value.getValue();
 }

src/control/control.h

@@ -11,25 +11,22 @@
 #include "control/controlvalue.h"
 #include "configobject.h"
 
+class ControlObject;
+
 class ControlDoublePrivate : public QObject {
     Q_OBJECT
   public:
-    ControlDoublePrivate();
-    ControlDoublePrivate(ConfigKey key, bool bIgnoreNops, bool bTrack);
     virtual ~ControlDoublePrivate();
 
     // Gets the ControlDoublePrivate matching the given ConfigKey. If bCreate
     // is true, allocates a new ControlDoublePrivate for the ConfigKey if one
     // does not exist.
-    static ControlDoublePrivate* getControl(
-        const ConfigKey& key,
-        bool bCreate, bool bIgnoreNops=true, bool bTrack=false);
-    static inline ControlDoublePrivate* getControl(
-        const QString& group, const QString& item,
-        bool bCreate, bool bIgnoreNops=true, bool bTrack=false) {
-        ConfigKey key(group, item);
-        return getControl(key, bCreate, bIgnoreNops, bTrack);
-    }
+    static QSharedPointer<ControlDoublePrivate> getControl(
+            const ConfigKey& key,
+            ControlObject* pCreatorCO = NULL, bool bIgnoreNops = true, bool bTrack = false);
+
+    // Adds all ControlDoublePrivate that currently exist to pControlList
+    static void getControls(QList<ControlDoublePrivate*>* pControlsList);
 
     // Sets the control value.
     void set(double value, QObject* pSender);
@@ -60,11 +57,24 @@ class ControlDoublePrivate : public QObject {
     inline void setDefaultValue(double dValue) {
         m_defaultValue.setValue(dValue);
     }
+
     inline double defaultValue() const {
         double default_value = m_defaultValue.getValue();
         return m_pBehavior ? m_pBehavior->defaultValue(default_value) : default_value;
     }
 
+    inline ControlObject* getCreatorCO() const {
+        return m_pCreatorCO;
+    }
+
+    inline void removeCreatorCO() {
+        m_pCreatorCO = NULL;
+    }
+
+    inline ConfigKey getKey() {
+        return m_key;
+    }
+
     // Connects a slot to the ValueChange request for CO validation. All change
     // requests issued by set are routed though the connected slot. This can
     // decide with its own thread safe solution if the requested value can be
@@ -81,6 +91,7 @@ class ControlDoublePrivate : public QObject {
     void valueChangeRequest(double value);
 
   private:
+    ControlDoublePrivate(ConfigKey key, ControlObject* pCreatorCO, bool bIgnoreNops, bool bTrack);
     void initialize();
     void setInner(double value, QObject* pSender);
 
@@ -102,11 +113,13 @@ class ControlDoublePrivate : public QObject {
 
     QSharedPointer<ControlNumericBehavior> m_pBehavior;
 
+    ControlObject* m_pCreatorCO;
+
     // Hash of ControlDoublePrivate instantiations.
-    static QHash<ConfigKey,ControlDoublePrivate*> m_sqCOHash;
+    static QHash<ConfigKey, QWeakPointer<ControlDoublePrivate> > m_sqCOHash;
     // Mutex guarding access to the ControlDoublePrivate hash.
     static QMutex m_sqCOHashMutex;
 };
 
 
-#endif /* CONTROL_H */
+#endif /* CONTROL_H */

src/controlobject.cpp

@@ -26,11 +26,6 @@
 #include "util/stat.h"
 #include "util/timer.h"
 
-// Static member variable definition
-QHash<ConfigKey,ControlObject*> ControlObject::m_sqCOHash;
-QMutex ControlObject::m_sqCOHashMutex;
-
-
 ControlObject::ControlObject()
         : m_pControl(NULL) {
 }
@@ -41,21 +36,15 @@ ControlObject::ControlObject(ConfigKey key, bool bIgnoreNops, bool bTrack)
 }
 
 ControlObject::~ControlObject() {
-    m_sqCOHashMutex.lock();
-    m_sqCOHash.remove(m_key);
-    m_sqCOHashMutex.unlock();
+    m_pControl->removeCreatorCO();
 }
 
 void ControlObject::initialize(ConfigKey key, bool bIgnoreNops, bool bTrack) {
     m_key = key;
-    m_pControl = ControlDoublePrivate::getControl(m_key, true, bIgnoreNops, bTrack);
-    connect(m_pControl, SIGNAL(valueChanged(double, QObject*)),
+    m_pControl = ControlDoublePrivate::getControl(m_key, this, bIgnoreNops, bTrack);
+    connect(m_pControl.data(), SIGNAL(valueChanged(double, QObject*)),
             this, SLOT(privateValueChanged(double, QObject*)),
             Qt::DirectConnection);
-
-    m_sqCOHashMutex.lock();
-    m_sqCOHash.insert(m_key, this);
-    m_sqCOHashMutex.unlock();
 }
 
 // slot
@@ -68,56 +57,13 @@ void ControlObject::privateValueChanged(double dValue, QObject* pSender) {
     }
 }
 
-/*
-bool ControlObject::connectControls(ConfigKey src, ConfigKey dest)
-{
-    // Find src and dest objects
-    ControlObject * pSrc = getControl(src);
-    ControlObject * pDest = getControl(dest);
-
-    if (pSrc && pDest) {
-        connect(pSrc, SIGNAL(valueChanged(double)), pDest, SLOT(set(double)));
-        return true;
-    } else {
-        return false;
-    }
-}
-
-bool ControlObject::disconnectControl(ConfigKey key)
-{
-    // Find src and dest objects
-    ControlObject * pSrc = getControl(key);
-
-    if (pSrc)
-    {
-        disconnect(pSrc, 0, 0, 0);
-        return true;
-    }
-    else
-        return false;
-}
-*/
-
-// static
-void ControlObject::getControls(QList<ControlObject*>* pControlList) {
-    m_sqCOHashMutex.lock();
-    for (QHash<ConfigKey, ControlObject*>::const_iterator it = m_sqCOHash.begin();
-         it != m_sqCOHash.end(); ++it) {
-        pControlList->push_back(it.value());
-    }
-    m_sqCOHashMutex.unlock();
-}
-
 // static
 ControlObject* ControlObject::getControl(const ConfigKey& key) {
     //qDebug() << "ControlObject::getControl for (" << key.group << "," << key.item << ")";
-    QMutexLocker locker(&m_sqCOHashMutex);
-    QHash<ConfigKey, ControlObject*>::const_iterator it = m_sqCOHash.find(key);
-    if (it != m_sqCOHash.end()) {
-        ControlObject* co = it.value();
-        return co;
+    QSharedPointer<ControlDoublePrivate> pCDP = ControlDoublePrivate::getControl(key);
+    if (pCDP) {
+        return pCDP->getCreatorCO();
     }
-    qWarning() << "ControlObject::getControl returning NULL for (" << key.group << "," << key.item << ")";
     return NULL;
 }
 
@@ -137,7 +83,7 @@ double ControlObject::get() const {
 
 // static
 double ControlObject::get(const ConfigKey& key) {
-    ControlDoublePrivate* pCop = ControlDoublePrivate::getControl(key, false);
+    QSharedPointer<ControlDoublePrivate> pCop = ControlDoublePrivate::getControl(key);
     return pCop ? pCop->get() : 0.0;
 }
 
@@ -161,7 +107,7 @@ void ControlObject::setAndConfirm(double value) {
 
 // static
 void ControlObject::set(const ConfigKey& key, const double& value) {
-    ControlDoublePrivate* pCop = ControlDoublePrivate::getControl(key, false);
+    QSharedPointer<ControlDoublePrivate> pCop = ControlDoublePrivate::getControl(key);
     if (pCop) {
         pCop->set(value, NULL);
     }

src/controlobject.h

@@ -45,9 +45,6 @@ class ControlObject : public QObject {
         return getControl(key);
     }
 
-    // Adds all ControlObjects that currently exist to pControlList
-    static void getControls(QList<ControlObject*>* pControlsList);
-
     // Return the key of the object
     inline ConfigKey getKey() const { return m_key; }
     // Returns the value of the ControlObject
@@ -90,7 +87,7 @@ class ControlObject : public QObject {
   protected:
     // Key of the object
     ConfigKey m_key;
-    ControlDoublePrivate* m_pControl;
+    QSharedPointer<ControlDoublePrivate> m_pControl;
 
   private slots:
     void privateValueChanged(double value, QObject* pSetter);
@@ -100,11 +97,6 @@ class ControlObject : public QObject {
     inline bool ignoreNops() const {
         return m_pControl ? m_pControl->ignoreNops() : true;
     }
-
-    // Hash of ControlObject instantiations
-    static QHash<ConfigKey,ControlObject*> m_sqCOHash;
-    // Mutex guarding access to the ControlObject hash
-    static QMutex m_sqCOHashMutex;
 };
 
 #endif

src/controlobjectthread.cpp

@@ -38,9 +38,9 @@ ControlObjectThread::ControlObjectThread(const ConfigKey& key, QObject* pParent)
 
 void ControlObjectThread::initialize(const ConfigKey& key) {
     m_key = key;
-    m_pControl = ControlDoublePrivate::getControl(m_key, false);
+    m_pControl = ControlDoublePrivate::getControl(key);
     if (m_pControl) {
-        connect(m_pControl, SIGNAL(valueChanged(double, QObject*)),
+        connect(m_pControl.data(), SIGNAL(valueChanged(double, QObject*)),
                 this, SLOT(slotValueChanged(double, QObject*)),
                 Qt::DirectConnection);
     }

src/controlobjectthread.h

@@ -69,7 +69,7 @@ class ControlObjectThread : public QObject {
   protected:
     ConfigKey m_key;
     // Pointer to connected control.
-    ControlDoublePrivate* m_pControl;
+    QSharedPointer<ControlDoublePrivate> m_pControl;
 };
 
 #endif