Bug 1642738, part 1 - Make the non-reply MessageChannel Send messages take a UniquePtr. r=froydnj

This fixes the leak of a ShmemCreated in the case where ShareHandle fails.

Differential Revision: https://phabricator.services.mozilla.com/D77907

ipc/glue/MessageChannel.cpp

@@ -941,7 +941,7 @@ bool MessageChannel::OpenOnSameThread(MessageChannel* aTargetChan,
   return true;
 }
 
-bool MessageChannel::Send(Message* aMsg) {
+bool MessageChannel::Send(UniquePtr<Message> aMsg) {
   if (aMsg->size() >= kMinTelemetryMessageSize) {
     Telemetry::Accumulate(Telemetry::IPC_MESSAGE_SIZE2, aMsg->size());
   }
@@ -963,28 +963,27 @@ bool MessageChannel::Send(Message* aMsg) {
   MOZ_RELEASE_ASSERT(!aMsg->is_sync());
   MOZ_RELEASE_ASSERT(aMsg->nested_level() != IPC::Message::NESTED_INSIDE_SYNC);
 
-  CxxStackFrame frame(*this, OUT_MESSAGE, aMsg);
+  CxxStackFrame frame(*this, OUT_MESSAGE, aMsg.get());
 
-  UniquePtr<Message> msg(aMsg);
   AssertWorkerThread();
   mMonitor->AssertNotCurrentThreadOwns();
-  if (MSG_ROUTING_NONE == msg->routing_id()) {
+  if (MSG_ROUTING_NONE == aMsg->routing_id()) {
     ReportMessageRouteError("MessageChannel::Send");
     return false;
   }
 
-  if (msg->seqno() == 0) {
-    msg->set_seqno(NextSeqno());
+  if (aMsg->seqno() == 0) {
+    aMsg->set_seqno(NextSeqno());
   }
 
   MonitorAutoLock lock(*mMonitor);
   if (!Connected()) {
-    ReportConnectionError("MessageChannel", msg.get());
+    ReportConnectionError("MessageChannel", aMsg.get());
     return false;
   }
 
-  AddProfilerMarker(msg.get(), MessageDirection::eSending);
-  SendMessageToLink(std::move(msg));
+  AddProfilerMarker(aMsg.get(), MessageDirection::eSending);
+  SendMessageToLink(std::move(aMsg));
   return true;
 }
 

ipc/glue/MessageChannel.h

@@ -218,16 +218,16 @@ class MessageChannel : HasResultCodes, MessageLoop::DestructionObserver {
   ChannelFlags GetChannelFlags() { return mFlags; }
 
   // Asynchronously send a message to the other side of the channel
-  bool Send(Message* aMsg);
+  bool Send(UniquePtr<Message> aMsg);
 
   // Asynchronously send a message to the other side of the channel
   // and wait for asynchronous reply.
   template <typename Value>
-  void Send(Message* aMsg, ActorIdType aActorId,
+  void Send(UniquePtr<Message> aMsg, ActorIdType aActorId,
             ResolveCallback<Value>&& aResolve, RejectCallback&& aReject) {
     int32_t seqno = NextSeqno();
     aMsg->set_seqno(seqno);
-    if (!Send(aMsg)) {
+    if (!Send(std::move(aMsg))) {
       aReject(ResponseRejectReason::SendError);
       return;
     }

ipc/glue/ProtocolUtils.cpp

@@ -463,7 +463,7 @@ bool IProtocol::ChannelSend(IPC::Message* aMsg) {
     // NOTE: This send call failing can only occur during toplevel channel
     // teardown. As this is an async call, this isn't reasonable to predict or
     // respond to, so just drop the message on the floor silently.
-    GetIPCChannel()->Send(msg.release());
+    GetIPCChannel()->Send(std::move(msg));
     return true;
   }
 
@@ -699,12 +699,12 @@ Shmem::SharedMemory* IToplevelProtocol::CreateSharedMemory(
       OtherPid();
 #endif
 
-  Message* descriptor =
+  UniquePtr<Message> descriptor =
       shmem.ShareTo(Shmem::PrivateIPDLCaller(), pid, MSG_ROUTING_CONTROL);
   if (!descriptor) {
     return nullptr;
   }
-  Unused << GetIPCChannel()->Send(descriptor);
+  Unused << GetIPCChannel()->Send(std::move(descriptor));
 
   *aId = shmem.Id(Shmem::PrivateIPDLCaller());
   Shmem::SharedMemory* rawSegment = segment.get();
@@ -733,7 +733,7 @@ bool IToplevelProtocol::DestroySharedMemory(Shmem& shmem) {
     return false;
   }
 
-  Message* descriptor =
+  UniquePtr<Message> descriptor =
       shmem.UnshareFrom(Shmem::PrivateIPDLCaller(), MSG_ROUTING_CONTROL);
 
   MOZ_ASSERT(mShmemMap.Contains(aId),
@@ -743,11 +743,10 @@ bool IToplevelProtocol::DestroySharedMemory(Shmem& shmem) {
 
   MessageChannel* channel = GetIPCChannel();
   if (!channel->CanSend()) {
-    delete descriptor;
     return true;
   }
 
-  return descriptor && channel->Send(descriptor);
+  return descriptor && channel->Send(std::move(descriptor));
 }
 
 void IToplevelProtocol::DeallocShmems() {

ipc/glue/ProtocolUtils.h

@@ -289,7 +289,7 @@ class IProtocol : public HasResultCodes {
                    RejectCallback&& aReject) {
     UniquePtr<IPC::Message> msg(aMsg);
     if (CanSend()) {
-      GetIPCChannel()->Send(msg.release(), this, std::move(aResolve),
+      GetIPCChannel()->Send(std::move(msg), this, std::move(aResolve),
                             std::move(aReject));
     } else {
       NS_WARNING("IPC message discarded: actor cannot send");

ipc/glue/Shmem.cpp

@@ -416,12 +416,13 @@ void Shmem::Dealloc(PrivateIPDLCaller, SharedMemory* aSegment) {
 
 #endif  // if defined(DEBUG)
 
-IPC::Message* Shmem::ShareTo(PrivateIPDLCaller, base::ProcessId aTargetPid,
-                             int32_t routingId) {
+UniquePtr<IPC::Message> Shmem::ShareTo(PrivateIPDLCaller,
+                                       base::ProcessId aTargetPid,
+                                       int32_t routingId) {
   AssertInvariants();
 
-  IPC::Message* msg = new ShmemCreated(routingId, mId, mSize, mSegment->Type());
-  if (!mSegment->ShareHandle(aTargetPid, msg)) {
+  auto msg = MakeUnique<ShmemCreated>(routingId, mId, mSize, mSegment->Type());
+  if (!mSegment->ShareHandle(aTargetPid, msg.get())) {
     return nullptr;
   }
   // close the handle to the segment after it is shared
@@ -429,9 +430,10 @@ IPC::Message* Shmem::ShareTo(PrivateIPDLCaller, base::ProcessId aTargetPid,
   return msg;
 }
 
-IPC::Message* Shmem::UnshareFrom(PrivateIPDLCaller, int32_t routingId) {
+UniquePtr<IPC::Message> Shmem::UnshareFrom(PrivateIPDLCaller,
+                                           int32_t routingId) {
   AssertInvariants();
-  return new ShmemDestroyed(routingId, mId);
+  return MakeUnique<ShmemDestroyed>(routingId, mId);
 }
 
 void IPDLParamTraits<Shmem>::Write(IPC::Message* aMsg, IProtocol* aActor,

ipc/glue/Shmem.h

@@ -18,6 +18,7 @@
 #include "ipc/IPCMessageUtils.h"
 #include "mozilla/ipc/SharedMemory.h"
 #include "mozilla/ipc/IPDLParamTraits.h"
+#include "mozilla/UniquePtr.h"
 
 /**
  * |Shmem| is one agent in the IPDL shared memory scheme.  The way it
@@ -158,14 +159,14 @@ class Shmem final {
   // that contains enough information for the other process to map
   // this segment in OpenExisting() below.  Return a new message if
   // successful (owned by the caller), nullptr if not.
-  IPC::Message* ShareTo(PrivateIPDLCaller, base::ProcessId aTargetPid,
-                        int32_t routingId);
+  UniquePtr<IPC::Message> ShareTo(PrivateIPDLCaller, base::ProcessId aTargetPid,
+                                  int32_t routingId);
 
   // Stop sharing this with |aTargetPid|.  Return an IPC message that
   // contains enough information for the other process to unmap this
   // segment.  Return a new message if successful (owned by the
   // caller), nullptr if not.
-  IPC::Message* UnshareFrom(PrivateIPDLCaller, int32_t routingId);
+  UniquePtr<IPC::Message> UnshareFrom(PrivateIPDLCaller, int32_t routingId);
 
   // Return a SharedMemory instance in this process using the
   // descriptor shared to us by the process that created the