dbus: Fix a subtle butterfly-effect bug in handling incoming messages

libdbus does bookkeeping of the number of bytes in the incoming message
queue implicitly, and asks client code (chrome) to stop monitoring the
underlying socket, as soon as it exceeds a certain number, which is set
to 63MB as of now.

This caused a DCHECK failure (and could break the D-Bus message
dispatching with Release builds) because the bookkeeping happned on the
UI thread via a seemingly harmless call to dbus_message_unref(), but we
cannot stop monitoring from UI thread.

This patch fixes (or works around) the problem by deleting incoming
messages on D-Bus thread, so the bookkeeping is done on D-Bus thread.

Note that we don't have to change exported_object.cc, as the method call
message is deleted on the D-Bus thread in
ExportedObject::OnMethodCompleted()

The following is a stacktrace of the DCHECK failure. Here,
dbus::Response (method reply) is deleted on UI thread, that results
in a call to dbus::Bus::OnToggleWatch, which should only be called on
the D-Bus thread, hence crashed as a DCHECK failure


Backtrace:
	base::debug::StackTrace::StackTrace() [0x517972]
	logging::LogMessage::~LogMessage() [0x4b3a57] <- crashing because we are not
	base::ThreadRestrictions::AssertIOAllowed() [0x4f0b35]
	dbus::Bus::AssertOnDBusThread() [0x45ceb6] <- checking if we are on the right thread
	dbus::Bus::OnToggleWatch() [0x45d0c1]
	dbus::Bus::OnToggleWatchThunk() [0x45d45d]  <-- the change is notified.
	_dbus_watch_list_toggle_watch [0x7f35e0a15245]
	protected_change_watch [0x7f35e09f2eef]
	_dbus_connection_toggle_watch_unlocked [0x7f35e09f302e]
	check_read_watch [0x7f35e0a1332d]   <-- what? why checking socket status here??
	socket_live_messages_changed [0x7f35e0a1436c]
	live_messages_size_notify [0x7f35e0a11996]
	_dbus_counter_adjust [0x7f35e0a0c098]
	free_size_counter [0x7f35e0a04423]
	_dbus_list_foreach [0x7f35e0a180d9]
	dbus_message_cache_or_finalize [0x7f35e0a0446b]
	dbus_message_unref [0x7f35e0a05e7e]   <-- releasing a message
	dbus::Message::~Message() [0x46abbb]
	dbus::Response::~Response() [0x470478]
	scoped_ptr<>::~scoped_ptr() [0x41e99f]
	dbus::ObjectProxy::RunResponseCallback() [0x472095]

BUG=126217
TEST=added unit tests

Review URL: https://chromiumcodereview.appspot.com/10492005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@140165 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 35 insertions, 0 deletions

diff --git a/dbus/end_to_end_async_unittest.cc b/dbus/end_to_end_async_unittest.cc
index d5e278f..7715f7d 100644
--- a/dbus/end_to_end_async_unittest.cc
+++ b/dbus/end_to_end_async_unittest.cc
@@ -20,6 +20,14 @@
 #include "dbus/test_service.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
+// See comments in ObjectProxy::RunResponseCallback() for why the number was
+// chosen.
+const int kHugePayloadSize = 64 << 20;  // 64 MB
+
+}  // namespace
+
 // The end-to-end test exercises the asynchronous APIs in ObjectProxy and
 // ExportedObject.
 class EndToEndAsyncTest : public testing::Test {
@@ -313,6 +321,23 @@ TEST_F(EndToEndAsyncTest, EchoThreeTimes) {
   EXPECT_EQ("foo", response_strings_[2]);
 }
 
+TEST_F(EndToEndAsyncTest, Echo_HugePayload) {
+  const std::string kHugePayload(kHugePayloadSize, 'o');
+
+  // Create the method call with a huge payload.
+  dbus::MethodCall method_call("org.chromium.TestInterface", "Echo");
+  dbus::MessageWriter writer(&method_call);
+  writer.AppendString(kHugePayload);
+
+  // Call the method.
+  const int timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT;
+  CallMethod(&method_call, timeout_ms);
+
+  // This caused a DCHECK failure before. Ensure that the issue is fixed.
+  WaitForResponses(1);
+  EXPECT_EQ(kHugePayload, response_strings_[0]);
+}
+
 TEST_F(EndToEndAsyncTest, BrokenBus) {
   const char* kHello = "hello";
 
@@ -537,6 +562,16 @@ TEST_F(EndToEndAsyncTest, TestSignalFromRoot) {
   ASSERT_EQ(kMessage, root_test_signal_string_);
 }
 
+TEST_F(EndToEndAsyncTest, TestHugeSignal) {
+  const std::string kHugeMessage(kHugePayloadSize, 'o');
+
+  // Send the huge signal from the exported object.
+  test_service_->SendTestSignal(kHugeMessage);
+  // This caused a DCHECK failure before. Ensure that the issue is fixed.
+  WaitForTestSignal();
+  ASSERT_EQ(kHugeMessage, test_signal_string_);
+}
+
 class SignalReplacementTest : public EndToEndAsyncTest {
  public:
   SignalReplacementTest() {