Mojo: Ensure partially serialized message handles don't leak

For the sake of efficiency, we don't fully serialize handles in a
message until we're ready to freeze the message contents for
transmission. Unfortunately we were not properly closing these
partially serialized handles in the event that the message was
never fully serialized before destruction. This fixes that.

R=jcivelli@chromium.org

Bug: 775592
Change-Id: Idac3b11359dc2fcec0699cef779de809575ea163
Reviewed-on: https://chromium-review.googlesource.com/723771Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509600}

mojo/edk/system/message_unittest.cc

@@ -899,6 +899,35 @@ DEFINE_TEST_CLIENT_TEST_WITH_PIPE(ReadMessageAndCheckPipe, MessageTest, h) {
 
 #endif  // !defined(OS_IOS)
 
+TEST_F(MessageTest, PartiallySerializedMessagesDontLeakHandles) {
+  MojoMessageHandle message;
+  EXPECT_EQ(MOJO_RESULT_OK, MojoCreateMessage(&message));
+
+  MojoHandle handles[2];
+  CreateMessagePipe(&handles[0], &handles[1]);
+
+  const std::string kTestMessagePart1("hello i am message.");
+  void* buffer;
+  uint32_t buffer_size;
+  EXPECT_EQ(MOJO_RESULT_OK,
+            MojoAttachSerializedMessageBuffer(
+                message, static_cast<uint32_t>(kTestMessagePart1.size()),
+                nullptr, 0, &buffer, &buffer_size));
+  ASSERT_GE(buffer_size, static_cast<uint32_t>(kTestMessagePart1.size()));
+  memcpy(buffer, kTestMessagePart1.data(), kTestMessagePart1.size());
+
+  EXPECT_EQ(MOJO_RESULT_OK,
+            MojoExtendSerializedMessagePayload(
+                message, static_cast<uint32_t>(kTestMessagePart1.size()),
+                handles, 1, &buffer, &buffer_size));
+
+  // This must close |handles[0]|, which we can detect by observing the
+  // signal state of |handles[1].
+  EXPECT_EQ(MOJO_RESULT_OK, MojoDestroyMessage(message));
+  EXPECT_EQ(MOJO_RESULT_OK,
+            WaitForSignals(handles[1], MOJO_HANDLE_SIGNAL_PEER_CLOSED));
+}
+
 }  // namespace
 }  // namespace edk
 }  // namespace mojo

mojo/edk/system/user_message_impl.cc

@@ -279,7 +279,9 @@ UserMessageImpl::~UserMessageImpl() {
 
     if (!pending_handle_attachments_.empty()) {
       internal::g_core->ReleaseDispatchersForTransit(
-          pending_handle_attachments_, true);
+          pending_handle_attachments_, false);
+      for (const auto& dispatcher : pending_handle_attachments_)
+        MojoClose(dispatcher.local_handle);
     }
   }
 }