From 41ece522bbb0cd5679e204acc8d980c6adf9ff67 Mon Sep 17 00:00:00 2001
From: hashimoto <hashimoto@chromium.org>
Date: Mon, 30 Mar 2015 00:01:39 -0700
Subject: dbus: Cancel pending calls when destroying ObjectProxy

After this change, it is guaranteed that no callback is called for method calls after
RemoveObjectProxy() is called.

BUG=471175
TEST=dbus_unittests

Review URL: https://codereview.chromium.org/1040683002

Cr-Commit-Position: refs/heads/master@{#322747}
---
 dbus/end_to_end_async_unittest.cc | 27 +++++++++++++++++++++++++++
 dbus/object_proxy.cc              | 20 +++++++++++++++-----
 dbus/object_proxy.h               |  2 ++
 3 files changed, 44 insertions(+), 5 deletions(-)

(limited to 'dbus')

diff --git a/dbus/end_to_end_async_unittest.cc b/dbus/end_to_end_async_unittest.cc
index 8846830..10516b9 100644
--- a/dbus/end_to_end_async_unittest.cc
+++ b/dbus/end_to_end_async_unittest.cc
@@ -418,6 +418,33 @@ TEST_F(EndToEndAsyncTest, TimeoutWithErrorCallback) {
   ASSERT_EQ(DBUS_ERROR_NO_REPLY, error_names_[0]);
 }
 
+TEST_F(EndToEndAsyncTest, CancelPendingCalls) {
+  const char* kHello = "hello";
+
+  // Create the method call.
+  MethodCall method_call("org.chromium.TestInterface", "Echo");
+  MessageWriter writer(&method_call);
+  writer.AppendString(kHello);
+
+  // Call the method.
+  const int timeout_ms = ObjectProxy::TIMEOUT_USE_DEFAULT;
+  CallMethod(&method_call, timeout_ms);
+
+  // Remove the object proxy before receiving the result.
+  // This results in cancelling the pending method call.
+  bus_->RemoveObjectProxy("org.chromium.TestService",
+                          ObjectPath("/org/chromium/TestObject"),
+                          base::Bind(&base::DoNothing));
+
+  // We shouldn't receive any responses. Wait for a while just to make sure.
+  run_loop_.reset(new base::RunLoop);
+  message_loop_.PostDelayedTask(FROM_HERE,
+                                run_loop_->QuitClosure(),
+                                TestTimeouts::tiny_timeout());
+  run_loop_->Run();
+  EXPECT_TRUE(response_strings_.empty());
+}
+
 // Tests calling a method that sends its reply asynchronously.
 TEST_F(EndToEndAsyncTest, AsyncEcho) {
   const char* kHello = "hello";
diff --git a/dbus/object_proxy.cc b/dbus/object_proxy.cc
index 974b24c..441dc75 100644
--- a/dbus/object_proxy.cc
+++ b/dbus/object_proxy.cc
@@ -60,6 +60,7 @@ ObjectProxy::ObjectProxy(Bus* bus,
 }
 
 ObjectProxy::~ObjectProxy() {
+  DCHECK(pending_calls_.empty());
 }
 
 // Originally we tried to make |method_call| a const reference, but we
@@ -207,16 +208,21 @@ void ObjectProxy::Detach() {
   if (bus_->is_connected())
     bus_->RemoveFilterFunction(&ObjectProxy::HandleMessageThunk, this);
 
-  for (std::set<std::string>::iterator iter = match_rules_.begin();
-       iter != match_rules_.end(); ++iter) {
+  for (const auto& match_rule : match_rules_) {
     ScopedDBusError error;
-    bus_->RemoveMatch(*iter, error.get());
+    bus_->RemoveMatch(match_rule, error.get());
     if (error.is_set()) {
       // There is nothing we can do to recover, so just print the error.
-      LOG(ERROR) << "Failed to remove match rule: " << *iter;
+      LOG(ERROR) << "Failed to remove match rule: " << match_rule;
     }
   }
   match_rules_.clear();
+
+  for (auto* pending_call : pending_calls_) {
+    dbus_pending_call_cancel(pending_call);
+    dbus_pending_call_unref(pending_call);
+  }
+  pending_calls_.clear();
 }
 
 // static
@@ -277,7 +283,7 @@ void ObjectProxy::StartAsyncMethodCall(int timeout_ms,
       data,
       &DeleteVoidPointer<OnPendingCallIsCompleteData>);
   CHECK(success) << "Unable to allocate memory";
-  dbus_pending_call_unref(pending_call);
+  pending_calls_.insert(pending_call);
 
   // It's now safe to unref the request message.
   dbus_message_unref(request_message);
@@ -297,6 +303,10 @@ void ObjectProxy::OnPendingCallIsComplete(DBusPendingCall* pending_call,
                                   start_time,
                                   response_message);
   bus_->GetOriginTaskRunner()->PostTask(FROM_HERE, task);
+
+  // Remove the pending call from the set.
+  pending_calls_.erase(pending_call);
+  dbus_pending_call_unref(pending_call);
 }
 
 void ObjectProxy::RunResponseCallback(ResponseCallback response_callback,
diff --git a/dbus/object_proxy.h b/dbus/object_proxy.h
index 2c9ef51..c0211b1 100644
--- a/dbus/object_proxy.h
+++ b/dbus/object_proxy.h
@@ -316,6 +316,8 @@ class CHROME_DBUS_EXPORT ObjectProxy
   // Known name owner of the well-known bus name represented by |service_name_|.
   std::string service_name_owner_;
 
+  std::set<DBusPendingCall*> pending_calls_;
+
   DISALLOW_COPY_AND_ASSIGN(ObjectProxy);
 };
 
-- 
cgit v1.1