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

src/service.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.h"
 
 #include <glog/logging.h>
 
 #include "src/data_plan_provider.h"
 #include "src/device.h"
@@ skipping 42 matching lines @@
 // Chromium OS Usage API status values
 // NOTE: add new values to IsValidCrosUsageStatus below also
 static const char *kCrosUsageStatusOk = "OK";
 static const char *kCrosUsageStatusError = "ERROR";
 static const char *kCrosUsageStatusMalformedRequest = "MALFORMED REQUEST";
 static const char *kCrosUsageStatusInternalError = "INTERNAL ERROR";
 static const char *kCrosUsageStatusServiceUnavailable = "SERVICE UNAVAILABLE";
 static const char *kCrosUsageStatusRequestRefused = "REQUEST REFUSED";
 static const char *kCrosUsageStatusUnknownDevice = "UNKNOWN DEVICE";
 
+// GetServiceProperties retry interval
+static const guint kSecondsPerMinute = 60;
+static const guint kGetServicePropertiesIntervalSeconds = 1 * kSecondsPerMinute;
+
 Service::Service(ServiceManager * const parent,
                  DBus::Connection& connection,  // NOLINT
                  const DBus::Path& path)
     : DBus::ObjectProxy(connection, path, kFlimflamServiceName),
       parent_(CHECK_NOTNULL(parent)), connection_(connection), path_(path),
       state_(kStateUnknown), type_(kTypeUnknown), device_(NULL),
       provider_(NULL), request_in_progress_(false),
       update_timeout_source_(NULL), policy_(NULL),
-      is_default_service_(false) {
-  // init our state with a GetProperties() call to our Flimflam service path
-  // we'll update this state by monitoring PropertyChanged signals
-  GetServiceProperties();
+      is_default_service_(false), 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(StaticGetServicePropertiesCallback, this);
+  if (get_properties_source_id_ == 0) {
+    LOG(ERROR) << path_ << ": ctor: g_idle_add failed";
+  }
 }
 
 Service::~Service() {
   DeleteCarrierState();
   DeleteDataPlans(&data_plans_);
   if (device_ != NULL) {
     DLOG(INFO) << path_ << ": deleting device " << device_->GetPath();
     delete device_;
     device_ = NULL;
   }
+  if (get_properties_source_id_ != 0 &&
+      !g_source_remove(get_properties_source_id_)) {
+    DLOG(WARNING) << path_ << ": dtor: g_source_remove failed";
+  }
 }
 
 const DBus::Path& Service::GetPath() const {
   return path_;
 }
 
 Service::State Service::GetState() const {
   return state_;
 }
 
@@ skipping 246 matching lines @@
 void Service::OnFlimflamOnline() {
   DLOG(INFO) << path_ << ": OnFlimflamOnline";
   ReconsiderSendingUsageRequests();
 }
 
 void Service::OnFlimflamOffline() {
   DLOG(INFO) << path_ << ": OnFlimflamOffline";
   ReconsiderSendingUsageRequests();
 }
 
+// glib integration interface
+
+guint Service::GetGetPropertiesSourceId() const {
+  return get_properties_source_id_;
+}
+
+void Service::SetGetPropertiesSourceId(guint source_id) {
+  get_properties_source_id_ = source_id;
+}
+
+bool Service::RetryingGetProperties() const {
+  return retrying_get_properties_;
+}
+
+void Service::OnRetryingGetProperties(bool retrying) {
+  retrying_get_properties_ = retrying;
+}
+
 // Private methods
 
 Service::State Service::StateFromString(const std::string& state) const {
   if (state == kFlimflamServiceStateIdle) {
     return kStateIdle;
   }
   if (state == kFlimflamServiceStateCarrier) {
     return kStateCarrier;
   }
   if (state == kFlimflamServiceStateAssociation) {
@@ skipping 71 matching lines @@
   }
   usage_url_ = usage_url;
   if (provider_ != NULL) {
     DCHECK(usage_url_ != provider_->GetUsageUrl());
     StopSendingUsageRequests();
     provider_->SetUsageUrl(usage_url_);
     ReconsiderSendingUsageRequests();
   }
 }
 
-void Service::GetServiceProperties() {
+// static
+gboolean Service::StaticGetServicePropertiesCallback(gpointer data) {
+  Service *service = reinterpret_cast<Service*>(data);
+  CHECK_NOTNULL(service);
+  DCHECK_NE(service->GetGetPropertiesSourceId(), 0);
+  if (!service->GetServiceProperties()) {
+    // call failed, so try again later
+    DLOG(WARNING) << service->GetPath()
+        << ": StaticGetServicePropertiesCallback: "
+        << "GetServiceProperties failed, will retry in "
+        << kGetServicePropertiesIntervalSeconds << " secs";
+    // if we've arrived here from our first g_idle_add call, set up a timer
+    if (!service->RetryingGetProperties()) {
+      guint source_id =
+          g_timeout_add_seconds(kGetServicePropertiesIntervalSeconds,
+                                StaticGetServicePropertiesCallback, service);
+      service->SetGetPropertiesSourceId(source_id);
+      if (source_id == 0) {
+        LOG(ERROR) << service->GetPath()
+            << ": StaticGetServicePropertiesCallback: "
+            << "g_timeout_add_seconds failed";
+        return FALSE;  // get rid of old glib source and give up
+      }
+      service->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->SetGetPropertiesSourceId(0);
+  service->OnRetryingGetProperties(false);
+  return FALSE;  // we succeeded, so don't repeat
+}
+
+bool Service::GetServiceProperties() {
   DLOG(INFO) << path_ << ": GetServiceProperties";
   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) << path_
         << ": GetServiceProperties: GetProperties() -> Exception: "
         << error.name() << ": " << error.message();
-    // TODO(vlaviano): schedule another attempt later
-    return;
+    return false;
   } catch (...) {  // NOLINT
     LOG(WARNING) << path_
         << ": GetServiceProperties: GetProperties() -> Exception";
-    // TODO(vlaviano): schedule another attempt later
-    return;
+    return false;
   }
   DLOG(INFO) << path_ << ": GetServiceProperties: Received "
       << properties.size() << " properties";
 
   // grab the properties in which we're interested
   PropertyMap::const_iterator it;
   it = properties.find(kFlimflamServiceDeviceProperty);
   if (it != properties.end()) {
     const DBus::Variant& value = static_cast<DBus::Variant>(it->second);
     OnDeviceUpdate(value.reader().get_path());
   } else {
     DLOG(WARNING) << path_ << ": GetServiceProperties: no Device property";
   }
   it = properties.find(kFlimflamServiceStateProperty);
   if (it != properties.end()) {
     const DBus::Variant& value = static_cast<DBus::Variant>(it->second);
     OnStateUpdate(value.reader().get_string());
   } else {
     DLOG(WARNING) << path_ << ": GetServiceProperties: no State property";
   }
   it = properties.find(kFlimflamServiceTypeProperty);
   if (it != properties.end()) {
     const DBus::Variant& value = static_cast<DBus::Variant>(it->second);
     OnTypeUpdate(value.reader().get_string());
   } else {
     DLOG(WARNING) << path_ << ": GetServiceProperties: no Type property";
   }
   // don't expect to find Cellular.* properties if we're not a cellular service
   if (type_ != kTypeCellular) {
-    return;
+    return true;
   }
   it = properties.find(kFlimflamServiceUsageUrlProperty);
   if (it != properties.end()) {
     const DBus::Variant& value = static_cast<DBus::Variant>(it->second);
     OnUsageUrlUpdate(value.reader().get_string());
   } else {
     DLOG(WARNING) << path_
         << ": GetServiceProperties: no Cellular.UsageUrl property";
   }
+  return true;
 }
 
 void Service::DeleteDataPlans(DataPlanList *data_plans) {
   DCHECK(data_plans != NULL);
   while (!data_plans->empty()) {
     DataPlan *plan = *data_plans->begin();
     DCHECK(plan != NULL);
     DLOG(INFO) << path_ << ": DeleteDataPlans: deleting plan: "
         << plan->GetName();
     delete plan;
@@ skipping 46 matching lines @@
 }
 
 gboolean Service::UpdateTimeoutCallback() {
   DLOG(INFO) << path_ << ": UpdateTimeoutCallback";
   RequestUsageUpdate();
   return TRUE;
 }
 
 // static
 gboolean Service::StaticUpdateTimeoutCallback(gpointer data) {
-  CHECK_NOTNULL(data);
-  return reinterpret_cast<Service*>(data)->UpdateTimeoutCallback();
+  Service *service = reinterpret_cast<Service*>(data);
+  CHECK_NOTNULL(service);
+  return service->UpdateTimeoutCallback();
 }
 
 bool Service::CreateUpdateTimer() {
   DCHECK(provider_ != NULL);
   DCHECK(policy_ != NULL);
   guint idle_seconds = policy_->GetUpdateTimerIdleSecs(data_plans_);
   DLOG(INFO) << path_ << ": CreateUpdateTimer: idle_seconds = " << idle_seconds;
   if (update_timeout_source_ != NULL) {
     DLOG(WARNING) << path_ << ": CreateUpdateTimer: timer already running";
     return false;
@@ skipping 134 matching lines @@
 void Service::ReconsiderSendingUsageRequests() {
   DLOG(INFO) << path_ << ": ReconsiderSendingUsageRequests";
   if (!IsSendingUsageRequests() && ShouldSendUsageRequests()) {
     StartSendingUsageRequests();
   } else if (IsSendingUsageRequests() && !ShouldSendUsageRequests()) {
     StopSendingUsageRequests();
   }
 }
 
 }  // namespace cashew