Delete D-Bus clients before shutting down the system bus

chromeos/dbus/dbus_thread_manager.cc

Index: chromeos/dbus/dbus_thread_manager.cc
diff --git a/chromeos/dbus/dbus_thread_manager.cc b/chromeos/dbus/dbus_thread_manager.cc
index 91a474d0014b8f40204929223c216f5e8877c434..e772c9a134809ba446161d3a548a227f5da46b23 100644
--- a/chromeos/dbus/dbus_thread_manager.cc
+++ b/chromeos/dbus/dbus_thread_manager.cc
@@ -53,6 +53,92 @@ namespace chromeos {
 static DBusThreadManager* g_dbus_thread_manager = NULL;
 static DBusThreadManager* g_dbus_thread_manager_for_testing = NULL;
 
+// The bundle of all D-Bus clients used in DBusThreadManagerImpl. The bundle
+// is used to delete them at once in the right order before shutting down the
+// system bus. See also the comment in the destructor of DBusThreadManagerImpl.
+struct DBusClientBundle {

[stevenjb, 2014/02/18 17:37:03]

Since this has non-trivial behavior we should make it a class.

[satorux1, 2014/02/19 04:16:31]

Done.

+  DBusClientBundle() {
+    DBusClientImplementationType client_type = REAL_DBUS_CLIENT_IMPLEMENTATION;
+    DBusClientImplementationType client_type_override =
+        REAL_DBUS_CLIENT_IMPLEMENTATION;

[stevenjb, 2014/02/18 17:37:03]

= client_type might be more clear?

[satorux1, 2014/02/19 04:16:31]

Done.

+    // If --dbus-stub was requested, pass STUB to specific components;
+    // Many components like login are not useful with a stub implementation.
+    if (CommandLine::ForCurrentProcess()->HasSwitch(
+            chromeos::switches::kDbusStub)) {
+      client_type_override = STUB_DBUS_CLIENT_IMPLEMENTATION;
+    }
+
+    bluetooth_adapter_client.reset(BluetoothAdapterClient::Create());
+    bluetooth_agent_manager_client.reset(
+        BluetoothAgentManagerClient::Create());
+    bluetooth_device_client.reset(BluetoothDeviceClient::Create());
+    bluetooth_input_client.reset(BluetoothInputClient::Create());
+    bluetooth_profile_manager_client.reset(
+        BluetoothProfileManagerClient::Create());
+    cras_audio_client.reset(CrasAudioClient::Create());
+    cros_disks_client.reset(CrosDisksClient::Create(client_type));
+    cryptohome_client.reset(CryptohomeClient::Create());
+    debug_daemon_client.reset(DebugDaemonClient::Create());
+    shill_manager_client.reset(ShillManagerClient::Create());
+    shill_device_client.reset(ShillDeviceClient::Create());
+    shill_ipconfig_client.reset(ShillIPConfigClient::Create());
+    shill_service_client.reset(ShillServiceClient::Create());
+    shill_profile_client.reset(ShillProfileClient::Create());
+    gsm_sms_client.reset(GsmSMSClient::Create());
+    image_burner_client.reset(ImageBurnerClient::Create());
+    introspectable_client.reset(IntrospectableClient::Create());
+    modem_messaging_client.reset(ModemMessagingClient::Create());
+    // Create the NFC clients in the correct order based on their dependencies.
+    nfc_manager_client.reset(NfcManagerClient::Create());
+    nfc_adapter_client.reset(
+        NfcAdapterClient::Create(nfc_manager_client.get()));
+    nfc_device_client.reset(
+        NfcDeviceClient::Create(nfc_adapter_client.get()));
+    nfc_tag_client.reset(NfcTagClient::Create(nfc_adapter_client.get()));
+    nfc_record_client.reset(NfcRecordClient::Create(nfc_device_client.get(),
+                                                    nfc_tag_client.get()));
+    permission_broker_client.reset(PermissionBrokerClient::Create());
+    power_manager_client.reset(
+        PowerManagerClient::Create(client_type_override));
+    session_manager_client.reset(SessionManagerClient::Create(client_type));
+    sms_client.reset(SMSClient::Create());
+    system_clock_client.reset(SystemClockClient::Create());
+    update_engine_client.reset(UpdateEngineClient::Create(client_type));
+  }
+
+  scoped_ptr<BluetoothAdapterClient> bluetooth_adapter_client;
+  scoped_ptr<BluetoothAgentManagerClient> bluetooth_agent_manager_client;
+  scoped_ptr<BluetoothDeviceClient> bluetooth_device_client;
+  scoped_ptr<BluetoothInputClient> bluetooth_input_client;
+  scoped_ptr<BluetoothProfileManagerClient> bluetooth_profile_manager_client;
+  scoped_ptr<CrasAudioClient> cras_audio_client;
+  scoped_ptr<CrosDisksClient> cros_disks_client;
+  scoped_ptr<CryptohomeClient> cryptohome_client;
+  scoped_ptr<DebugDaemonClient> debug_daemon_client;
+  scoped_ptr<ShillDeviceClient> shill_device_client;
+  scoped_ptr<ShillIPConfigClient> shill_ipconfig_client;
+  scoped_ptr<ShillManagerClient> shill_manager_client;
+  scoped_ptr<ShillServiceClient> shill_service_client;
+  scoped_ptr<ShillProfileClient> shill_profile_client;
+  scoped_ptr<GsmSMSClient> gsm_sms_client;
+  scoped_ptr<ImageBurnerClient> image_burner_client;
+  scoped_ptr<IntrospectableClient> introspectable_client;
+  scoped_ptr<ModemMessagingClient> modem_messaging_client;
+  // The declaration order for NFC client objects is important. See
+  // DBusThreadManager::CreateDefaultClients for the dependencies.
+  scoped_ptr<NfcManagerClient> nfc_manager_client;
+  scoped_ptr<NfcAdapterClient> nfc_adapter_client;
+  scoped_ptr<NfcDeviceClient> nfc_device_client;
+  scoped_ptr<NfcTagClient> nfc_tag_client;
+  scoped_ptr<NfcRecordClient> nfc_record_client;
+  scoped_ptr<PermissionBrokerClient> permission_broker_client;
+  scoped_ptr<SystemClockClient> system_clock_client;
+  scoped_ptr<PowerManagerClient> power_manager_client;
+  scoped_ptr<SessionManagerClient> session_manager_client;
+  scoped_ptr<SMSClient> sms_client;
+  scoped_ptr<UpdateEngineClient> update_engine_client;
+};
+
 // The DBusThreadManager implementation used in production.
 class DBusThreadManagerImpl : public DBusThreadManager {
  public:
@@ -77,6 +163,9 @@ class DBusThreadManagerImpl : public DBusThreadManager {
     FOR_EACH_OBSERVER(DBusThreadManagerObserver, observers_,
                       OnDBusThreadManagerDestroying(this));
 
+    // Delete all D-Bus clients before shutting down the system bus.
+    client_bundle_.reset();
+
     // Shut down the bus. During the browser shutdown, it's ok to shut down
     // the bus synchronously.
     system_bus_->ShutdownOnDBusThreadAndBlock();
@@ -101,178 +190,132 @@ class DBusThreadManagerImpl : public DBusThreadManager {
   }
 
   virtual BluetoothAdapterClient* GetBluetoothAdapterClient() OVERRIDE {
-    return bluetooth_adapter_client_.get();
+    return client_bundle_->bluetooth_adapter_client.get();
   }
 
   virtual BluetoothAgentManagerClient* GetBluetoothAgentManagerClient()
       OVERRIDE {
-    return bluetooth_agent_manager_client_.get();
+    return client_bundle_->bluetooth_agent_manager_client.get();
   }
 
   virtual BluetoothDeviceClient* GetBluetoothDeviceClient() OVERRIDE {
-    return bluetooth_device_client_.get();
+    return client_bundle_->bluetooth_device_client.get();
   }
 
   virtual BluetoothInputClient* GetBluetoothInputClient() OVERRIDE {
-    return bluetooth_input_client_.get();
+    return client_bundle_->bluetooth_input_client.get();
   }
 
   virtual BluetoothProfileManagerClient* GetBluetoothProfileManagerClient()
       OVERRIDE {
-    return bluetooth_profile_manager_client_.get();
+    return client_bundle_->bluetooth_profile_manager_client.get();
   }
 
   virtual CrasAudioClient* GetCrasAudioClient() OVERRIDE {
-    return cras_audio_client_.get();
+    return client_bundle_->cras_audio_client.get();
   }
 
   virtual CrosDisksClient* GetCrosDisksClient() OVERRIDE {
-    return cros_disks_client_.get();
+    return client_bundle_->cros_disks_client.get();
   }
 
   virtual CryptohomeClient* GetCryptohomeClient() OVERRIDE {
-    return cryptohome_client_.get();
+    return client_bundle_->cryptohome_client.get();
   }
 
   virtual DebugDaemonClient* GetDebugDaemonClient() OVERRIDE {
-    return debug_daemon_client_.get();
+    return client_bundle_->debug_daemon_client.get();
   }
 
   virtual ShillDeviceClient* GetShillDeviceClient() OVERRIDE {
-    return shill_device_client_.get();
+    return client_bundle_->shill_device_client.get();
   }
 
   virtual ShillIPConfigClient* GetShillIPConfigClient() OVERRIDE {
-    return shill_ipconfig_client_.get();
+    return client_bundle_->shill_ipconfig_client.get();
   }
 
   virtual ShillManagerClient* GetShillManagerClient() OVERRIDE {
-    return shill_manager_client_.get();
+    return client_bundle_->shill_manager_client.get();
   }
 
   virtual ShillServiceClient* GetShillServiceClient() OVERRIDE {
-    return shill_service_client_.get();
+    return client_bundle_->shill_service_client.get();
   }
 
   virtual ShillProfileClient* GetShillProfileClient() OVERRIDE {
-    return shill_profile_client_.get();
+    return client_bundle_->shill_profile_client.get();
   }
 
   virtual GsmSMSClient* GetGsmSMSClient() OVERRIDE {
-    return gsm_sms_client_.get();
+    return client_bundle_->gsm_sms_client.get();
   }
 
   virtual ImageBurnerClient* GetImageBurnerClient() OVERRIDE {
-    return image_burner_client_.get();
+    return client_bundle_->image_burner_client.get();
   }
 
   virtual IntrospectableClient* GetIntrospectableClient() OVERRIDE {
-    return introspectable_client_.get();
+    return client_bundle_->introspectable_client.get();
   }
 
   virtual ModemMessagingClient* GetModemMessagingClient() OVERRIDE {
-    return modem_messaging_client_.get();
+    return client_bundle_->modem_messaging_client.get();
   }
 
   virtual NfcAdapterClient* GetNfcAdapterClient() OVERRIDE {
-    return nfc_adapter_client_.get();
+    return client_bundle_->nfc_adapter_client.get();
   }
 
   virtual NfcDeviceClient* GetNfcDeviceClient() OVERRIDE {
-    return nfc_device_client_.get();
+    return client_bundle_->nfc_device_client.get();
   }
 
   virtual NfcManagerClient* GetNfcManagerClient() OVERRIDE {
-    return nfc_manager_client_.get();
+    return client_bundle_->nfc_manager_client.get();
   }
 
   virtual NfcRecordClient* GetNfcRecordClient() OVERRIDE {
-    return nfc_record_client_.get();
+    return client_bundle_->nfc_record_client.get();
   }
 
   virtual NfcTagClient* GetNfcTagClient() OVERRIDE {
-    return nfc_tag_client_.get();
+    return client_bundle_->nfc_tag_client.get();
   }
 
   virtual PermissionBrokerClient* GetPermissionBrokerClient() OVERRIDE {
-    return permission_broker_client_.get();
+    return client_bundle_->permission_broker_client.get();
   }
 
   virtual PowerManagerClient* GetPowerManagerClient() OVERRIDE {
-    return power_manager_client_.get();
-  }
-
-  virtual PowerPolicyController* GetPowerPolicyController() OVERRIDE {
-    return power_policy_controller_.get();
+    return client_bundle_->power_manager_client.get();
   }
 
   virtual SessionManagerClient* GetSessionManagerClient() OVERRIDE {
-    return session_manager_client_.get();
+    return client_bundle_->session_manager_client.get();
   }
 
   virtual SMSClient* GetSMSClient() OVERRIDE {
-    return sms_client_.get();
+    return client_bundle_->sms_client.get();
   }
 
   virtual SystemClockClient* GetSystemClockClient() OVERRIDE {
-    return system_clock_client_.get();
+    return client_bundle_->system_clock_client.get();
   }
 
   virtual UpdateEngineClient* GetUpdateEngineClient() OVERRIDE {
-    return update_engine_client_.get();
+    return client_bundle_->update_engine_client.get();
+  }
+
+  virtual PowerPolicyController* GetPowerPolicyController() OVERRIDE {
+    return power_policy_controller_.get();
   }
 
  private:
   // Constructs all clients and stores them in the respective *_client_ member
   // variable.
   void CreateDefaultClients() {
-    DBusClientImplementationType client_type = REAL_DBUS_CLIENT_IMPLEMENTATION;
-    DBusClientImplementationType client_type_override =
-        REAL_DBUS_CLIENT_IMPLEMENTATION;
-    // If --dbus-stub was requested, pass STUB to specific components;
-    // Many components like login are not useful with a stub implementation.
-    if (CommandLine::ForCurrentProcess()->HasSwitch(
-            chromeos::switches::kDbusStub)) {
-      client_type_override = STUB_DBUS_CLIENT_IMPLEMENTATION;
-    }
-
-    bluetooth_adapter_client_.reset(BluetoothAdapterClient::Create());
-    bluetooth_agent_manager_client_.reset(
-        BluetoothAgentManagerClient::Create());
-    bluetooth_device_client_.reset(BluetoothDeviceClient::Create());
-    bluetooth_input_client_.reset(BluetoothInputClient::Create());
-    bluetooth_profile_manager_client_.reset(
-        BluetoothProfileManagerClient::Create());
-    cras_audio_client_.reset(CrasAudioClient::Create());
-    cros_disks_client_.reset(CrosDisksClient::Create(client_type));
-    cryptohome_client_.reset(CryptohomeClient::Create());
-    debug_daemon_client_.reset(DebugDaemonClient::Create());
-    shill_manager_client_.reset(ShillManagerClient::Create());
-    shill_device_client_.reset(ShillDeviceClient::Create());
-    shill_ipconfig_client_.reset(ShillIPConfigClient::Create());
-    shill_service_client_.reset(ShillServiceClient::Create());
-    shill_profile_client_.reset(ShillProfileClient::Create());
-    gsm_sms_client_.reset(GsmSMSClient::Create());
-    image_burner_client_.reset(ImageBurnerClient::Create());
-    introspectable_client_.reset(IntrospectableClient::Create());
-    modem_messaging_client_.reset(ModemMessagingClient::Create());
-    // Create the NFC clients in the correct order based on their dependencies.
-    nfc_manager_client_.reset(NfcManagerClient::Create());
-    nfc_adapter_client_.reset(
-        NfcAdapterClient::Create(nfc_manager_client_.get()));
-    nfc_device_client_.reset(
-        NfcDeviceClient::Create(nfc_adapter_client_.get()));
-    nfc_tag_client_.reset(NfcTagClient::Create(nfc_adapter_client_.get()));
-    nfc_record_client_.reset(NfcRecordClient::Create(nfc_device_client_.get(),
-                                                     nfc_tag_client_.get()));
-    permission_broker_client_.reset(PermissionBrokerClient::Create());
-    power_manager_client_.reset(
-        PowerManagerClient::Create(client_type_override));
-    session_manager_client_.reset(SessionManagerClient::Create(client_type));
-    sms_client_.reset(SMSClient::Create());
-    system_clock_client_.reset(SystemClockClient::Create());
-    update_engine_client_.reset(UpdateEngineClient::Create(client_type));
-
+    client_bundle_.reset(new DBusClientBundle);
     power_policy_controller_.reset(new PowerPolicyController);
   }
 
@@ -282,38 +325,7 @@ class DBusThreadManagerImpl : public DBusThreadManager {
 
   scoped_ptr<base::Thread> dbus_thread_;
   scoped_refptr<dbus::Bus> system_bus_;
-  scoped_ptr<BluetoothAdapterClient> bluetooth_adapter_client_;
-  scoped_ptr<BluetoothAgentManagerClient> bluetooth_agent_manager_client_;
-  scoped_ptr<BluetoothDeviceClient> bluetooth_device_client_;
-  scoped_ptr<BluetoothInputClient> bluetooth_input_client_;
-  scoped_ptr<BluetoothProfileManagerClient> bluetooth_profile_manager_client_;
-  scoped_ptr<CrasAudioClient> cras_audio_client_;
-  scoped_ptr<CrosDisksClient> cros_disks_client_;
-  scoped_ptr<CryptohomeClient> cryptohome_client_;
-  scoped_ptr<DebugDaemonClient> debug_daemon_client_;
-  scoped_ptr<ShillDeviceClient> shill_device_client_;
-  scoped_ptr<ShillIPConfigClient> shill_ipconfig_client_;
-  scoped_ptr<ShillManagerClient> shill_manager_client_;
-  scoped_ptr<ShillServiceClient> shill_service_client_;
-  scoped_ptr<ShillProfileClient> shill_profile_client_;
-  scoped_ptr<GsmSMSClient> gsm_sms_client_;
-  scoped_ptr<ImageBurnerClient> image_burner_client_;
-  scoped_ptr<IntrospectableClient> introspectable_client_;
-  scoped_ptr<ModemMessagingClient> modem_messaging_client_;
-  // The declaration order for NFC client objects is important. See
-  // DBusThreadManager::CreateDefaultClients for the dependencies.
-  scoped_ptr<NfcManagerClient> nfc_manager_client_;
-  scoped_ptr<NfcAdapterClient> nfc_adapter_client_;
-  scoped_ptr<NfcDeviceClient> nfc_device_client_;
-  scoped_ptr<NfcTagClient> nfc_tag_client_;
-  scoped_ptr<NfcRecordClient> nfc_record_client_;
-  scoped_ptr<PermissionBrokerClient> permission_broker_client_;
-  scoped_ptr<SystemClockClient> system_clock_client_;
-  scoped_ptr<PowerManagerClient> power_manager_client_;
-  scoped_ptr<SessionManagerClient> session_manager_client_;
-  scoped_ptr<SMSClient> sms_client_;
-  scoped_ptr<UpdateEngineClient> update_engine_client_;
-
+  scoped_ptr<DBusClientBundle> client_bundle_;
   scoped_ptr<PowerPolicyController> power_policy_controller_;
 };