Revert "cashew: do not delete DBus::ObjectProxy objects from D-Bus callbacks"

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 65 matching lines @@
 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), get_properties_source_id_(0),
       retrying_get_properties_(false),
-      connectivity_state_(kConnectivityStateUnknown),
-      deferred_device_deletion_source_id_(0) {
+      connectivity_state_(kConnectivityStateUnknown) {
   // 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_);
-  // ServiceManager ensures that this dtor is not being called from within a
-  // D-Bus callback, so it's ok to delete |device_| and any members of
-  // |devices_pending_deletion_| directly.
   if (device_ != NULL) {
-    DLOG(INFO) << path_ << ": dtor: deleting device " << device_->GetPath();
+    DLOG(INFO) << path_ << ": deleting device " << device_->GetPath();
     delete device_;
     device_ = NULL;
   }
-  DeletePendingDevices();
   if (get_properties_source_id_ != 0 &&
       !g_source_remove(get_properties_source_id_)) {
-    DLOG(WARNING) << path_ << ": dtor: g_source_remove failed for source id "
-        << get_properties_source_id_;
-  }
-  if (deferred_device_deletion_source_id_ != 0 &&
-      !g_source_remove(deferred_device_deletion_source_id_)) {
-    DLOG(WARNING) << path_ << ": dtor: g_source_remove failed for source id "
-        << deferred_device_deletion_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 325 matching lines @@
 }
 
 bool Service::RetryingGetProperties() const {
   return retrying_get_properties_;
 }
 
 void Service::OnRetryingGetProperties(bool retrying) {
   retrying_get_properties_ = retrying;
 }
 
-guint Service::GetDeferredDeviceDeletionSourceId() const {
-  return deferred_device_deletion_source_id_;
-}
-
-void Service::SetDeferredDeviceDeletionSourceId(guint source_id) {
-  deferred_device_deletion_source_id_ = source_id;
-}
-
 // Private methods
 
-void Service::DeletePendingDevices() {
-  while (!devices_pending_deletion_.empty()) {
-    Device *device = *(devices_pending_deletion_.begin());
-    DCHECK(device != NULL);
-    DLOG(INFO) << path_ << ": DeletePendingDevices: deleting device: "
-        << device->GetPath();
-    DCHECK(device != device_);
-    delete device;
-    devices_pending_deletion_.erase(devices_pending_deletion_.begin());
-  }
-}
-
-void Service::ScheduleDeviceForLaterDeletion(Device *device) {
-  DCHECK(device != NULL);
-  DCHECK(device != device_);
-  // We might be inside of a D-Bus callback. We add the device to our
-  // |devices_pending_deletion_| list and schedule a glib callback to
-  // delete these devices from the main loop if one is not already
-  // scheduled.
-  DLOG(INFO) << path_ << ": ScheduleDeviceForLaterDeletion: "
-      << device->GetPath();
-  devices_pending_deletion_.push_back(device);
-  if (deferred_device_deletion_source_id_ != 0) {
-    DLOG(INFO) << path_ << ": ScheduleDeviceForLaterDeletion: "
-        << "deferred device deletion callback already scheduled";
-    return;
-  }
-  deferred_device_deletion_source_id_ =
-      g_idle_add(StaticDeletePendingDevicesCallback, this);
-  if (deferred_device_deletion_source_id_ == 0) {
-    LOG(ERROR) << path_
-        << ": ScheduleDeviceForLaterDeletion: g_idle_add failed";
-    return;
-  }
-  DLOG(INFO) << path_ << ": ScheduleDeviceForLaterDeletion: "
-      << "scheduled deferred device deletion callback";
-}
-
-// static
-gboolean Service::StaticDeletePendingDevicesCallback(gpointer data) {
-  Service *service = reinterpret_cast<Service*>(data);
-  DCHECK(service != NULL);
-  DCHECK_NE(service->GetDeferredDeviceDeletionSourceId(), 0);
-  service->DeletePendingDevices();
-  // we don't want to be run again automatically
-  // ScheduleDeviceForLaterDeletion will schedule us as needed
-  service->SetDeferredDeviceDeletionSourceId(0);
-  return FALSE;
-}
-
 // static
 Service::ConnectivityState Service::ConnectivityStateFromString(
     const std::string& connectivity_state) {
   if (connectivity_state == kFlimflamServiceConnectivityStateUnknown) {
     return kConnectivityStateUnknown;
   }
   if (connectivity_state == kFlimflamServiceConnectivityStateRestricted) {
     return kConnectivityStateRestricted;
   }
   if (connectivity_state == kFlimflamServiceConnectivityStateUnrestricted) {
@@ skipping 35 matching lines @@
 }
 
 void Service::OnDeviceUpdate(const DBus::Path& device_path) {
   DLOG(INFO) << path_ << ": OnDeviceUpdate: device_path = " << device_path;
 
   // if there's an existing device with a non-matching path: destroy it and
   // fall through to make a new one below.
   if (device_ != NULL && device_->GetPath() != device_path) {
     LOG(WARNING) << path_ << ": OnDeviceUpdate: device path changed from "
         << device_->GetPath() << " to " << device_path;
-    // We might be in a D-Bus callback, so schedule device for later deletion
-    // to avoid possible deadlock. See the comments in service_manager.h and
-    // service_manager.cc for more on this issue.
-    Device *device_to_delete = device_;
+    delete device_;
     device_ = NULL;
-    ScheduleDeviceForLaterDeletion(device_to_delete);
   }
 
   // if there's an existing device with matching path: all is well
   if (device_ != NULL) {
     return;
   }
 
   // if there's no existing device: make one
   device_ = new(std::nothrow) Device(this, connection_, device_path);
   if (device_ == NULL) {
@@ skipping 393 matching lines @@
   DCHECK(policy_ != NULL);
   if (policy_->ShouldEmitDataPlansUpdate(data_plans_)) {
     DLOG(INFO) << path_ << ": MaybeEmitDataPlansUpdate: sending update";
     parent_->EmitDataPlansUpdate(*this);
   } else {
     DLOG(INFO) << path_ << ": MaybeEmitDataPlansUpdate: not sending update";
   }
 }
 
 }  // namespace cashew