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

src/service_manager.cc

 // Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/service_manager.h"
 
 #include <glog/logging.h>
 
 #include "src/cashew_server.h"
 #include "src/service.h"
@@ skipping 10 matching lines @@
 static const char *kStateProperty = "State";
 
 // Flimflam on-the-wire State values
 static const char *kFlimflamStateOffline = "offline";
 static const char *kFlimflamStateConnected = "connected";
 static const char *kFlimflamStateOnline = "online";
 
 // Flimflam service types
 static const char *kTypeCellular = "cellular";
 
+// GetFlimflamProperties retry interval
+static const guint kSecondsPerMinute = 60;
+static const guint kGetFlimflamPropertiesIntervalSeconds =
+    1 * kSecondsPerMinute;
+
 ServiceManager::ServiceManager(DBus::Connection& connection)  // NOLINT
     : DBus::ObjectProxy(connection, kFlimflamManagerPath,
                         kFlimflamManagerName),
       connection_(connection), cashew_server_(NULL),
       default_technology_(Service::kTypeUnknown),
       default_cellular_service_(NULL),
-      connectivity_state_(kConnectivityStateUnknown) {
-  // init our state with a GetProperties() call to Flimflam
-  // we'll update this state by monitoring PropertyChanged signals
-  GetFlimflamProperties();
+      connectivity_state_(kConnectivityStateUnknown),
+      get_properties_source_id_(0), retrying_get_properties_(false) {
+  // schedule a GetProperties() call to Flimflam to init our 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(StaticGetFlimflamPropertiesCallback, this);
+  if (get_properties_source_id_ == 0) {
+    LOG(ERROR) << "ctor: g_idle_add failed";
+  }
 }
 
 ServiceManager::~ServiceManager() {
   ClearDefaultCellularService();
   DeleteServices(&services_);
+  if (get_properties_source_id_ != 0 &&
+      !g_source_remove(get_properties_source_id_)) {
+    DLOG(WARNING) << "dtor: g_source_remove failed";
+  }
 }
 
 const Service* ServiceManager::GetService(const std::string& service_path)
     const {
   ServiceMap::const_iterator it = services_.find(service_path);
   if (it == services_.end()) {
     return NULL;
   }
   DCHECK(it->second != NULL);
   return static_cast<const Service *>(it->second);
@@ skipping 73 matching lines @@
 
 void ServiceManager::EmitDataPlansUpdate(const Service& service) {
   DLOG(INFO) << "EmitDataPlansUpdate: service = " << service.GetPath();
   if (cashew_server_ == NULL) {
     DLOG(WARNING) << "EmitDataPlansUpdate: no Cashew server";
     return;
   }
   cashew_server_->EmitDataPlansUpdate(service);
 }
 
+// glib integration interface
+
+guint ServiceManager::GetGetPropertiesSourceId() const {
+  return get_properties_source_id_;
+}
+
+void ServiceManager::SetGetPropertiesSourceId(guint source_id) {
+  get_properties_source_id_ = source_id;
+}
+
+bool ServiceManager::RetryingGetProperties() const {
+  return retrying_get_properties_;
+}
+
+void ServiceManager::OnRetryingGetProperties(bool retrying) {
+  retrying_get_properties_ = retrying;
+}
+
 // Private methods
 
 void ServiceManager::DeleteServices(ServiceMap *service_map) {
   DCHECK(service_map != NULL);
   while (!service_map->empty()) {
     const std::string& path =
         static_cast<std::string>(service_map->begin()->first);
     DLOG(INFO) << "DeleteServices: deleting service: " << path;
     Service *service = static_cast<Service *>(service_map->begin()->second);
     DCHECK(service != NULL);
     DCHECK(service != default_cellular_service_);
     DCHECK(*service_map == services_ || GetService(service->GetPath()) == NULL);
     delete service;
     service_map->erase(service_map->begin());
   }
 }
 
-void ServiceManager::GetFlimflamProperties() {
+// static
+gboolean ServiceManager::StaticGetFlimflamPropertiesCallback(gpointer data) {
+  ServiceManager *service_manager = reinterpret_cast<ServiceManager*>(data);
+  CHECK_NOTNULL(service_manager);
+  DCHECK_NE(service_manager->GetGetPropertiesSourceId(), 0);
+  if (!service_manager->GetFlimflamProperties()) {
+    // call failed, so try again later
+    DLOG(WARNING) << "StaticGetFlimflamPropertiesCallback: "
+        << "GetFlimflamProperties failed, will retry in "
+        << kGetFlimflamPropertiesIntervalSeconds << " secs";
+    // if we've arrived here from our first g_idle_add call, set up a timer
+    if (!service_manager->RetryingGetProperties()) {
+      guint source_id =
+          g_timeout_add_seconds(kGetFlimflamPropertiesIntervalSeconds,
+                                StaticGetFlimflamPropertiesCallback,
+                                service_manager);
+      service_manager->SetGetPropertiesSourceId(source_id);
+      if (source_id == 0) {
+        LOG(ERROR) << "StaticGetFlimflamPropertiesCallback: "
+            << "g_timeout_add_seconds failed";
+        return FALSE;  // get rid of old glib source and give up
+      }
+      service_manager->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;
+  }
+  service_manager->SetGetPropertiesSourceId(0);
+  service_manager->OnRetryingGetProperties(false);
+  return FALSE;  // we succeeded, so don't repeat
+}
+
+bool ServiceManager::GetFlimflamProperties() {
   DLOG(INFO) << "GetFlimflamProperties";
   PropertyMap properties;
   // dbus-c++ throws exceptions
   // invoke the "Existing Non-conformant Code" clause of the style guide and
   // isolate the rest of the system from this
   try {
     // TODO(vlaviano): make this call asynchronous
     properties = GetProperties();
   } catch (DBus::Error& error) {  // NOLINT
     LOG(WARNING) << "GetFlimflamProperties: GetProperties() -> Exception: "
         << error.name() << ": " << error.message();
-    // TODO(vlaviano): schedule another attempt later
-    return;
+    return false;
   } catch (...) {  // NOLINT
     LOG(WARNING) << "GetFlimflamProperties: GetProperties() -> Exception";
-    // TODO(vlaviano): schedule another attempt later
-    return;
+    return false;
   }
   DLOG(INFO) << "GetFlimflamProperties: Received " << properties.size()
       << " properties";
 
   // grab the properties in which we're interested
   PropertyMap::const_iterator it = properties.find(kDefaultTechnologyProperty);
   if (it != properties.end()) {
     const DBus::Variant& value = static_cast<DBus::Variant>(it->second);
     OnDefaultTechnologyUpdate(value.reader().get_string());
   } else {
@@ skipping 10 matching lines @@
   } else {
     DLOG(WARNING) << "GetFlimflamProperties: no Services property";
   }
   it = properties.find(kStateProperty);
   if (it != properties.end()) {
     const DBus::Variant& value = static_cast<DBus::Variant>(it->second);
     StateChanged(value.reader().get_string());
   } else {
     DLOG(WARNING) << "GetFlimflamProperties: no State property";
   }
+  return true;
 }
 
 void ServiceManager::OnDefaultTechnologyUpdate(
     const std::string& default_technology) {
   DLOG(INFO) << "OnDefaultTechnologyUpdate: default technology = "
       << default_technology;
   default_technology_ = Service::TypeFromString(default_technology);
   if (default_technology_ == Service::kTypeUnknown) {
     LOG(WARNING) << "OnDefaultTechnologyUpdate: unknown default technology: "
         << default_technology;
@@ skipping 151 matching lines @@
   // notify our child services
   ServiceMap::iterator it;
   for (it = services_.begin(); it != services_.end(); ++it) {
     Service *service = static_cast<Service*>(it->second);
     DCHECK(service != NULL);
     service->OnFlimflamOffline();
   }
 }
 
 }  // namespace cashew