libcontextprovider, protocol change: Ref counting for PropertyPrivate; making PropertyPrivate's unregister

themselves from D-Bus when they are destructed. Documenting that you shouldn't destroy Service's
without destroying the associated Properties first.

6 files changed, 75 insertions, 5 deletions

diff --git a/libcontextprovider/src/property.cpp b/libcontextprovider/src/property.cpp
index 5cdf1029..b35d0d68 100644
--- a/libcontextprovider/src/property.cpp
+++ b/libcontextprovider/src/property.cpp
@@ -36,8 +36,10 @@ namespace ContextProvider {
     \brief A Property object represents a context property, i.e., a
     key-value pair.
 
-    Each Property object is associated with a Service object at
-    construction time.
+    Every Property object is associated with a Service object. If you
+    delete the Service object, the associated Property objects will
+    turn invalid and you should not use them.
+
 */
 
 /// Create a Property object on \a service for the key \a k.
@@ -71,9 +73,9 @@ void Property::init(ServiceBackend *serviceBackend, const QString &key)
     }
     else {
         priv = new PropertyPrivate(serviceBackend, key);
-        priv->serviceBackend->addProperty(priv->key, priv);
         PropertyPrivate::propertyPrivateMap.insert(lookup, priv);
     }
+    priv->ref();
     sconnect(priv, SIGNAL(firstSubscriberAppeared(const QString&)),
              this, SIGNAL(firstSubscriberAppeared(const QString&)));
     sconnect(priv, SIGNAL(lastSubscriberDisappeared(const QString&)),
@@ -116,6 +118,7 @@ QVariant Property::value()
 Property::~Property()
 {
     contextDebug() << F_PROPERTY << F_DESTROY << "Destroying Property for key:" << priv->key;
+    priv->unref();
 }
 
 } // end namespace
diff --git a/libcontextprovider/src/propertyprivate.cpp b/libcontextprovider/src/propertyprivate.cpp
index 5d4bb03c..94366d00 100644
--- a/libcontextprovider/src/propertyprivate.cpp
+++ b/libcontextprovider/src/propertyprivate.cpp
@@ -32,10 +32,47 @@ namespace ContextProvider {
 QHash<QPair<ServiceBackend*,QString>, PropertyPrivate*> PropertyPrivate::propertyPrivateMap;
 
 PropertyPrivate::PropertyPrivate(ServiceBackend* serviceBackend, const QString &key, QObject *parent)
-    : QObject(parent), serviceBackend(serviceBackend),
+    : QObject(parent), refCount(0), serviceBackend(serviceBackend),
       key(key), value(QVariant()),  timestamp(currentTimestamp()), subscribed(false),
       emittedValue(value), emittedTimestamp(timestamp), overheard(false)
 {
+    // Associate the property to the service backend
+    serviceBackend->addProperty(key, this);
+}
+
+PropertyPrivate::~PropertyPrivate()
+{
+    // Remove the property from the service backend
+    serviceBackend->removeProperty(key);
+}
+
+/// Increase the reference count by one. Property calls this.
+void PropertyPrivate::ref()
+{
+    refCount++;
+}
+
+/// Decrease the reference count by one. Property calls this. If the
+/// reference count goes to zero, schedule the PropertyPrivate
+/// instance to be deleted.
+void PropertyPrivate::unref()
+{
+    refCount--;
+
+    if (refCount == 0) {
+        // For now, remove the PropertyPrivate from the instance
+        // map. If this is changed, we need to check carefully what
+        // happens if a new ServiceBackend is created with the same
+        // memory address as a previous ServiceBackend (which was
+        // destructed), and having the old PropertyPrivate store the
+        // ServiceBackend pointer to that memory address.
+        QPair<ServiceBackend*, QString> key = propertyPrivateMap.key(this);
+        if (key.second != "")
+            propertyPrivateMap.remove(key);
+        else
+            contextCritical() << "PropertyPrivate couldn't find itself in the instance store";
+        deleteLater(); // "delete this" would be probably unsafe
+    }
 }
 
 void PropertyPrivate::setValue(const QVariant& v)
diff --git a/libcontextprovider/src/propertyprivate.h b/libcontextprovider/src/propertyprivate.h
index 647aed9d..73be67c4 100644
--- a/libcontextprovider/src/propertyprivate.h
+++ b/libcontextprovider/src/propertyprivate.h
@@ -37,6 +37,10 @@ class PropertyPrivate : public QObject
 
 public:
     explicit PropertyPrivate(ServiceBackend* serviceBackend, const QString &key, QObject *parent = 0);
+    virtual ~PropertyPrivate();
+
+    void ref();
+    void unref();
 
     void setValue(const QVariant& v);
     void updateOverheardValue(const QVariantList&, const quint64&);
@@ -52,6 +56,7 @@ private:
     static quint64 currentTimestamp();
     void emitValue();
 
+    int refCount;
     ServiceBackend* serviceBackend; ///< Pointer to the serviceBackend taking care of D-Bus related things
     QString key; ///< Key of this property
     QVariant value; ///< Current value of the property, set by this provider. QVariant() if null.
diff --git a/libcontextprovider/src/service.cpp b/libcontextprovider/src/service.cpp
index bbc5ea98..ad3e5d34 100644
--- a/libcontextprovider/src/service.cpp
+++ b/libcontextprovider/src/service.cpp
@@ -140,7 +140,9 @@ namespace ContextProvider {
     delete s2; // the "com.example.simple" just disappeared from D-Bus
     \endcode
 
-    Every Property object must be associated with a Service object.
+    Every Property object is associated with a Service object. If you
+    delete the Service object, the associated Property objects will
+    turn invalid and you should not use them.
 
     A Service can be running or stopped.  When it is running, it is
     visible via D-Bus and clients can subscribe to its properties.
diff --git a/libcontextprovider/src/servicebackend.cpp b/libcontextprovider/src/servicebackend.cpp
index 2e41feea..e9cf0cdc 100644
--- a/libcontextprovider/src/servicebackend.cpp
+++ b/libcontextprovider/src/servicebackend.cpp
@@ -72,6 +72,12 @@ ServiceBackend::~ServiceBackend()
 {
     contextDebug() << F_SERVICE_BACKEND << F_DESTROY << "Destroying Service";
     stop();
+
+    if (properties.size() > 0) {
+        contextCritical() << F_SERVICE_BACKEND << "Destroying a Service object "
+                          << "before destroying the associated Property objects.";
+    }
+
     if (ServiceBackend::defaultServiceBackend == this)
         ServiceBackend::defaultServiceBackend = 0;
 }
@@ -87,6 +93,8 @@ void ServiceBackend::setValue(const QString &key, const QVariant &v)
         contextWarning() << "Cannot set value for Property" << key << ", it does not exist";
 }
 
+/// Associate a PropertyPrivate object with this ServiceBackend. The
+/// corresponding object will appear on D-Bus.
 void ServiceBackend::addProperty(const QString& key, PropertyPrivate* property)
 {
     properties.insert(key, property);
@@ -94,6 +102,20 @@ void ServiceBackend::addProperty(const QString& key, PropertyPrivate* property)
     registerProperty(key, property);
 }
 
+// Break the association of the PropertyPrivate object with this
+// ServiceBackend. The corresponding object will disappear from D-Bus.
+void ServiceBackend::removeProperty(const QString& key)
+{
+    properties.remove(key);
+
+    // Unregister the object on D-Bus
+    PropertyAdaptor* adaptor = createdAdaptors.take(key);
+    if (adaptor != 0) {
+        connection.unregisterObject(adaptor->objectPath());
+        delete adaptor;
+    }
+}
+
 /// Register a Property with the given name on D-Bus. Returns true if
 /// succeeded, false if failed.
 bool ServiceBackend::registerProperty(const QString& key, PropertyPrivate* property)
diff --git a/libcontextprovider/src/servicebackend.h b/libcontextprovider/src/servicebackend.h
index 067fa8e6..dfee8087 100644
--- a/libcontextprovider/src/servicebackend.h
+++ b/libcontextprovider/src/servicebackend.h
@@ -51,6 +51,7 @@ public:
     void stop();
 
     void addProperty(const QString& key, PropertyPrivate* property);
+    void removeProperty(const QString& key);
 
     void setAsDefault();
     void setValue(const QString &key, const QVariant &val);