cashew: don't invoke D-Bus methods from within a D-Bus handler

src/device.cc

Index: src/device.cc
diff --git a/src/device.cc b/src/device.cc
index 80106352510e44de6f86c68e1e833364e2ffbb48..f85484bd7296343a70908950d774fb06eb3dd068 100644
--- a/src/device.cc
+++ b/src/device.cc
@@ -29,17 +29,31 @@ static const char* kFlimflamDeviceTypeCellular = "cellular";
 // TODO(vlaviano): what does vendor look like?
 static const char* kFlimflamDeviceTypeVendor = "";
 
+// GetDeviceProperties retry interval
+static const guint kSecondsPerMinute = 60;
+static const guint kGetDevicePropertiesIntervalSeconds = 1 * kSecondsPerMinute;
+
 Device::Device(Service * const parent, DBus::Connection& connection,  // NOLINT
                const DBus::Path& path)
     : DBus::ObjectProxy(connection, path, kFlimflamDeviceName),
       parent_(CHECK_NOTNULL(parent)), path_(path), type_(kTypeUnknown),
-      carrier_(kCarrierUnknown) {
-  // init our state with a GetProperties() call to our Flimflam service path
-  // we'll update this state by monitoring PropertyChanged signals
-  GetDeviceProperties();
+      carrier_(kCarrierUnknown), get_properties_source_id_(0),
+      retrying_get_properties_(false) {
+  // schedule a GetProperties() call to our Flimflam service path to init state
+  // we'll keep trying periodically until we succeed
+  // we'll subsequently update this state by monitoring PropertyChanged signals
+  get_properties_source_id_ =
+      g_idle_add(StaticGetDevicePropertiesCallback, this);
+  if (get_properties_source_id_ == 0) {
+    LOG(ERROR) << path_ << ": ctor: g_idle_add failed";
+  }
 }
 
 Device::~Device() {
+  if (get_properties_source_id_ != 0 &&
+      !g_source_remove(get_properties_source_id_)) {
+    DLOG(WARNING) << path_ << ": dtor: g_source_remove failed";
+  }
 }
 
 const DBus::Path& Device::GetPath() const {
@@ -55,6 +69,7 @@ const std::string& Device::GetCarrier() const {
 }
 
 // Flimflam Device D-Bus Proxy methods
+
 void Device::PropertyChanged(const std::string& property_name,
                              const DBus::Variant& new_value) {
   DLOG(INFO) << path_ << ": PropertyChanged: property_name = " << property_name;
@@ -67,6 +82,24 @@ void Device::PropertyChanged(const std::string& property_name,
   }
 }
 
+// glib integration interface
+
+guint Device::GetGetPropertiesSourceId() const {
+  return get_properties_source_id_;
+}
+
+void Device::SetGetPropertiesSourceId(guint source_id) {
+  get_properties_source_id_ = source_id;
+}
+
+bool Device::RetryingGetProperties() const {
+  return retrying_get_properties_;
+}
+
+void Device::OnRetryingGetProperties(bool retrying) {
+  retrying_get_properties_ = retrying;
+}
+
 // Private methods
 
 Device::Type Device::TypeFromString(const std::string& type) const {
@@ -116,7 +149,43 @@ void Device::OnTypeUpdate(const std::string& type) {
   }
 }
 
-void Device::GetDeviceProperties() {
+// static
+gboolean Device::StaticGetDevicePropertiesCallback(gpointer data) {
+  Device *device = reinterpret_cast<Device*>(data);
+  CHECK_NOTNULL(device);
+  DCHECK_NE(device->GetGetPropertiesSourceId(), 0);
+  if (!device->GetDeviceProperties()) {
+    // call failed, so try again later
+    DLOG(WARNING) << device->GetPath()
+        << ": StaticGetDevicePropertiesCallback: "
+        << "GetDeviceProperties failed, will retry in "
+        << kGetDevicePropertiesIntervalSeconds << " secs";
+    // if we've arrived here from our first g_idle_add call, set up a timer
+    if (!device->RetryingGetProperties()) {
+      guint source_id =
+          g_timeout_add_seconds(kGetDevicePropertiesIntervalSeconds,
+                                StaticGetDevicePropertiesCallback, device);
+      device->SetGetPropertiesSourceId(source_id);
+      if (source_id == 0) {
+          LOG(ERROR) << device->GetPath()
+              << ": StaticGetDevicePropertiesCallback: "
+              << "g_timeout_add_seconds failed";
+        return FALSE;  // get rid of old glib source and give up
+      }
+      device->OnRetryingGetProperties(true);
+      // get rid of our old glib source, but our new timer will call us
+      return FALSE;
+    }
+    // otherwise, our timer is already set up
+    // return TRUE to indicate that we want to be called again later
+    return TRUE;
+  }
+  device->SetGetPropertiesSourceId(0);
+  device->OnRetryingGetProperties(false);
+  return FALSE;  // we succeeded, so don't repeat
+}
+
+bool Device::GetDeviceProperties() {
   DLOG(INFO) << path_ << ": GetDeviceProperties";
   PropertyMap properties;
   // dbus-c++ throws exceptions
@@ -129,13 +198,11 @@ void Device::GetDeviceProperties() {
     LOG(WARNING) << path_
         << ": GetDeviceProperties: GetProperties() -> Exception: "
         << error.name() << ": " << error.message();
-    // TODO(vlaviano): schedule another attempt later
-    return;
+    return false;
   } catch (...) {  // NOLINT
     LOG(WARNING) << path_
         << ": GetDeviceProperties: GetProperties() -> Exception";
-    // TODO(vlaviano): schedule another attempt later
-    return;
+    return false;
   }
   DLOG(INFO) << "Received " << properties.size() << " properties";
 
@@ -150,7 +217,7 @@ void Device::GetDeviceProperties() {
   }
   // don't expect to find Cellular.* properties if we're not a cellular device
   if (type_ != kTypeCellular) {
-    return;
+    return true;
   }
   it = properties.find(kFlimflamDeviceCarrierProperty);
   if (it != properties.end()) {
@@ -160,6 +227,7 @@ void Device::GetDeviceProperties() {
     DLOG(WARNING) << path_
         << ": GetDeviceProperties: no Cellular.Carrier property";
   }
+  return true;
 }
 
 }  // namespace cashew