chromeos-dbus-bindings: Add destructor for ObjectManagerProxy

Previously, the generated ObjectManagerProxy was registering interfaces
with its wrapped ObjectManager instance but not unregistering them. The
registered interfaces were using the ObjectManagerProxy pointer to
create new property sets for the given interfaces. Unfortunately, this
led to a race where:

1) ObjectManagerProxy is destroyed
2) dbus::Bus object receives a signal that new interface has been added
3) ObjectManager instance previously wrapped by ObjectManagerProxy deferences
   the pointer to the ObjectManagerProxy (now invalid)
4) Terrible things happen here.

BUG=None
TEST=generated proxies still compile.

Change-Id: Ie1da0955088606d5cd41a1bffc131b114dc1f173
Reviewed-on: https://chromium-review.googlesource.com/236885
Commit-Queue: Christopher Wiley <wiley@chromium.org>
Tested-by: Christopher Wiley <wiley@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>

3 files changed, 32 insertions, 0 deletions

diff --git a/chromeos-dbus-bindings/proxy_generator.cc b/chromeos-dbus-bindings/proxy_generator.cc
index 66f39ea..797a7d5 100644
--- a/chromeos-dbus-bindings/proxy_generator.cc
+++ b/chromeos-dbus-bindings/proxy_generator.cc
@@ -553,6 +553,7 @@ void ProxyGenerator::ObjectManager::GenerateProxy(
   text->PushOffset(kBlockOffset);
 
   AddConstructor(config, class_name, interfaces, text);
+  AddDestructor(class_name, interfaces, text);
   AddGetObjectManagerProxy(text);
   for (const auto& itf : interfaces) {
     AddInterfaceAccessors(itf, text);
@@ -618,6 +619,22 @@ void ProxyGenerator::ObjectManager::AddConstructor(
   text->AddBlankLine();
 }
 
+void ProxyGenerator::ObjectManager::AddDestructor(
+    const std::string& class_name,
+    const std::vector<Interface>& interfaces,
+    IndentedText* text) {
+  text->AddLine(StringPrintf("~%s() override {", class_name.c_str()));
+  text->PushOffset(kBlockOffset);
+  for (const auto& itf : interfaces) {
+    text->AddLine(
+        StringPrintf("dbus_object_manager_->UnregisterInterface(\"%s\");",
+                     itf.name.c_str()));
+  }
+  text->PopOffset();
+  text->AddLine("}");
+  text->AddBlankLine();
+}
+
 void ProxyGenerator::ObjectManager::AddGetObjectManagerProxy(
     IndentedText* text) {
   text->AddLine("dbus::ObjectManager* GetObjectManagerProxy() const {");
diff --git a/chromeos-dbus-bindings/proxy_generator.h b/chromeos-dbus-bindings/proxy_generator.h
index 8fd46a4..682e6dc 100644
--- a/chromeos-dbus-bindings/proxy_generator.h
+++ b/chromeos-dbus-bindings/proxy_generator.h
@@ -101,6 +101,11 @@ class ProxyGenerator : public HeaderGenerator {
                                const std::vector<Interface>& interfaces,
                                IndentedText* text);
 
+    // Generates Object Manager destructor.
+    static void AddDestructor(const std::string& class_name,
+                              const std::vector<Interface>& interfaces,
+                              IndentedText* text);
+
     // Generates GetObjectManagerProxy() method.
     static void AddGetObjectManagerProxy(IndentedText* text);
 
diff --git a/chromeos-dbus-bindings/proxy_generator_unittest.cc b/chromeos-dbus-bindings/proxy_generator_unittest.cc
index 7251293..d374ee4 100644
--- a/chromeos-dbus-bindings/proxy_generator_unittest.cc
+++ b/chromeos-dbus-bindings/proxy_generator_unittest.cc
@@ -621,6 +621,11 @@ class ObjectManagerProxy : public dbus::ObjectManager::Interface {
     dbus_object_manager_->RegisterInterface("org.chromium.Itf2", this);
   }
 
+  ~ObjectManagerProxy() override {
+    dbus_object_manager_->UnregisterInterface("org.chromium.Itf1");
+    dbus_object_manager_->UnregisterInterface("org.chromium.Itf2");
+  }
+
   dbus::ObjectManager* GetObjectManagerProxy() const {
     return dbus_object_manager_;
   }
@@ -937,6 +942,11 @@ class ObjectManagerProxy : public dbus::ObjectManager::Interface {
     dbus_object_manager_->RegisterInterface("org.chromium.Itf2", this);
   }
 
+  ~ObjectManagerProxy() override {
+    dbus_object_manager_->UnregisterInterface("org.chromium.Itf1");
+    dbus_object_manager_->UnregisterInterface("org.chromium.Itf2");
+  }
+
   dbus::ObjectManager* GetObjectManagerProxy() const {
     return dbus_object_manager_;
   }