Bug 1121676 - Use a lock to protect the list of top-level actors (r=bent)
authorBill McCloskey <bill.mccloskey@gmail.com>
Thu, 26 Feb 2015 22:31:00 -0800
changeset 252101 ca7617c40ed6293e14bfd631d43fc6c7959d79bf
parent 252100 72c740d37eb9affb9af3673152a62a765a01b2bd
child 252102 5832c2042614f3831651193058847ce485e5e5c1
push id1174
push usernsm.nikhil@gmail.com
push dateSun, 22 Mar 2015 01:24:25 +0000
reviewersbent
bugs1121676
milestone39.0a1
Bug 1121676 - Use a lock to protect the list of top-level actors (r=bent)
dom/ipc/ContentChild.cpp
ipc/glue/ProtocolUtils.cpp
ipc/glue/ProtocolUtils.h
ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp
testing/mochitest/mochitest_options.py
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -546,29 +546,30 @@ InitOnContentProcessCreated()
     NS_WARN_IF(!smc);
 
     // This will register cross-process observer.
     mozilla::dom::time::InitializeDateCacheCleaner();
 }
 
 #ifdef MOZ_NUWA_PROCESS
 static void
-ResetTransports(void* aUnused) {
+ResetTransports(void* aUnused)
+{
   ContentChild* child = ContentChild::GetSingleton();
   mozilla::ipc::Transport* transport = child->GetTransport();
   int fd = transport->GetFileDescriptor();
   transport->ResetFileDescriptor(fd);
 
-  IToplevelProtocol* toplevel = child->GetFirstOpenedActors();
-  while (toplevel != nullptr) {
+  nsTArray<IToplevelProtocol*> actors;
+  child->GetOpenedActors(actors);
+  for (size_t i = 0; i < actors.Length(); i++) {
+      IToplevelProtocol* toplevel = actors[i];
       transport = toplevel->GetTransport();
       fd = transport->GetFileDescriptor();
       transport->ResetFileDescriptor(fd);
-
-      toplevel = toplevel->getNext();
   }
 }
 #endif
 
 #if defined(MOZ_TASK_TRACER) && defined(MOZ_NUWA_PROCESS)
 static void
 ReinitTaskTracer(void* /*aUnused*/)
 {
@@ -2672,19 +2673,20 @@ GetProtoFdInfos(NuwaProtoFdInfo* aInfoLi
 
     mozilla::dom::ContentChild* content =
         mozilla::dom::ContentChild::GetSingleton();
     aInfoList[i].protoId = content->GetProtocolId();
     aInfoList[i].originFd =
         content->GetTransport()->GetFileDescriptor();
     i++;
 
-    for (IToplevelProtocol* actor = content->GetFirstOpenedActors();
-         actor != nullptr;
-         actor = actor->getNext()) {
+    IToplevelProtocol* actors[NUWA_TOPLEVEL_MAX];
+    size_t count = content->GetOpenedActorsUnsafe(actors, ArrayLength(actors));
+    for (size_t j = 0; j < count; j++) {
+        IToplevelProtocol* actor = actors[j];
         if (i >= aInfoListSize) {
             NS_RUNTIMEABORT("Too many top level protocols!");
         }
 
         aInfoList[i].protoId = actor->GetProtocolId();
         aInfoList[i].originFd =
             actor->GetTransport()->GetFileDescriptor();
         i++;
--- a/ipc/glue/ProtocolUtils.cpp
+++ b/ipc/glue/ProtocolUtils.cpp
@@ -16,73 +16,145 @@ using namespace IPC;
 using base::GetCurrentProcessHandle;
 using base::GetProcId;
 using base::ProcessHandle;
 using base::ProcessId;
 
 namespace mozilla {
 namespace ipc {
 
-#ifdef MOZ_IPDL_TESTS
-bool IToplevelProtocol::sAllowNonMainThreadUse;
-#endif
+static Atomic<size_t> gNumProtocols;
+static StaticAutoPtr<Mutex> gProtocolMutex;
+
+IToplevelProtocol::IToplevelProtocol(ProtocolId aProtoId)
+ : mOpener(nullptr)
+ , mProtocolId(aProtoId)
+ , mTrans(nullptr)
+{
+  size_t old = gNumProtocols++;
+
+  if (!old) {
+    // We assume that two threads never race to create the first protocol. This
+    // assertion is sufficient to ensure that.
+    MOZ_ASSERT(NS_IsMainThread());
+    gProtocolMutex = new Mutex("ITopLevelProtocol::ProtocolMutex");
+  }
+}
 
 IToplevelProtocol::~IToplevelProtocol()
 {
-  MOZ_ASSERT(NS_IsMainThread() || AllowNonMainThreadUse());
-  mOpenActors.clear();
+  bool last = false;
+
+  {
+    MutexAutoLock al(*gProtocolMutex);
+
+    for (IToplevelProtocol* actor = mOpenActors.getFirst();
+         actor;
+         actor = actor->getNext()) {
+      actor->mOpener = nullptr;
+    }
+
+    mOpenActors.clear();
+
+    if (mOpener) {
+      removeFrom(mOpener->mOpenActors);
+    }
+
+    gNumProtocols--;
+    last = gNumProtocols == 0;
+  }
+
+  if (last) {
+    gProtocolMutex = nullptr;
+  }
 }
 
-void IToplevelProtocol::AddOpenedActor(IToplevelProtocol* aActor)
+void
+IToplevelProtocol::AddOpenedActorLocked(IToplevelProtocol* aActor)
 {
-  MOZ_ASSERT(NS_IsMainThread() || AllowNonMainThreadUse());
+  gProtocolMutex->AssertCurrentThreadOwns();
 
 #ifdef DEBUG
   for (const IToplevelProtocol* actor = mOpenActors.getFirst();
        actor;
        actor = actor->getNext()) {
     NS_ASSERTION(actor != aActor,
                  "Open the same protocol for more than one time");
   }
 #endif
 
+  aActor->mOpener = this;
   mOpenActors.insertBack(aActor);
 }
 
+void
+IToplevelProtocol::AddOpenedActor(IToplevelProtocol* aActor)
+{
+  MutexAutoLock al(*gProtocolMutex);
+  AddOpenedActorLocked(aActor);
+}
+
+void
+IToplevelProtocol::GetOpenedActorsLocked(nsTArray<IToplevelProtocol*>& aActors)
+{
+  gProtocolMutex->AssertCurrentThreadOwns();
+
+  for (IToplevelProtocol* actor = mOpenActors.getFirst();
+       actor;
+       actor = actor->getNext()) {
+    aActors.AppendElement(actor);
+  }
+}
+
+void
+IToplevelProtocol::GetOpenedActors(nsTArray<IToplevelProtocol*>& aActors)
+{
+  MutexAutoLock al(*gProtocolMutex);
+  GetOpenedActorsLocked(aActors);
+}
+
+size_t
+IToplevelProtocol::GetOpenedActorsUnsafe(IToplevelProtocol** aActors, size_t aActorsMax)
+{
+  size_t count = 0;
+  for (IToplevelProtocol* actor = mOpenActors.getFirst();
+       actor;
+       actor = actor->getNext()) {
+    MOZ_RELEASE_ASSERT(count < aActorsMax);
+    aActors[count++] = actor;
+  }
+  return count;
+}
+
 IToplevelProtocol*
 IToplevelProtocol::CloneToplevel(const InfallibleTArray<ProtocolFdMapping>& aFds,
                                  base::ProcessHandle aPeerProcess,
                                  ProtocolCloneContext* aCtx)
 {
   NS_NOTREACHED("Clone() for this protocol actor is not implemented");
   return nullptr;
 }
 
 void
 IToplevelProtocol::CloneOpenedToplevels(IToplevelProtocol* aTemplate,
                                         const InfallibleTArray<ProtocolFdMapping>& aFds,
                                         base::ProcessHandle aPeerProcess,
                                         ProtocolCloneContext* aCtx)
 {
-  for (IToplevelProtocol* actor = aTemplate->GetFirstOpenedActors();
-       actor;
-       actor = actor->getNext()) {
-    IToplevelProtocol* newactor = actor->CloneToplevel(aFds, aPeerProcess, aCtx);
-    AddOpenedActor(newactor);
+  MutexAutoLock al(*gProtocolMutex);
+
+  nsTArray<IToplevelProtocol*> actors;
+  aTemplate->GetOpenedActorsLocked(actors);
+
+  for (size_t i = 0; i < actors.Length(); i++) {
+    IToplevelProtocol* newactor = actors[i]->CloneToplevel(aFds, aPeerProcess, aCtx);
+    AddOpenedActorLocked(newactor);
   }
 }
 
-#ifdef MOZ_IPDL_TESTS
-void
-IToplevelProtocol::SetAllowNonMainThreadUse()
-{
-  sAllowNonMainThreadUse = true;
-}
-#endif
-
 class ChannelOpened : public IPC::Message
 {
 public:
   ChannelOpened(TransportDescriptor aDescriptor,
                 ProcessId aOtherProcess,
                 ProtocolId aProtocol,
                 PriorityValue aPriority = PRIORITY_NORMAL)
     : IPC::Message(MSG_ROUTING_CONTROL, // these only go to top-level actors
--- a/ipc/glue/ProtocolUtils.h
+++ b/ipc/glue/ProtocolUtils.h
@@ -16,16 +16,17 @@
 
 #include "IPCMessageStart.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/ipc/FileDescriptor.h"
 #include "mozilla/ipc/Shmem.h"
 #include "mozilla/ipc/Transport.h"
 #include "mozilla/ipc/MessageLink.h"
 #include "mozilla/LinkedList.h"
+#include "mozilla/Mutex.h"
 #include "MainThreadUtils.h"
 
 #if defined(ANDROID) && defined(DEBUG)
 #include <android/log.h>
 #endif
 
 // WARNING: this takes into account the private, special-message-type
 // enum in ipc_channel.h.  They need to be kept in sync.
@@ -176,26 +177,23 @@ public:
 };
 
 /**
  * All top-level protocols should inherit this class.
  *
  * IToplevelProtocol tracks all top-level protocol actors created from
  * this protocol actor.
  */
-class IToplevelProtocol : public LinkedListElement<IToplevelProtocol>
+class IToplevelProtocol : private LinkedListElement<IToplevelProtocol>
 {
+    friend class LinkedList<IToplevelProtocol>;
+    friend class LinkedListElement<IToplevelProtocol>;
+
 protected:
-    explicit IToplevelProtocol(ProtocolId aProtoId)
-        : mProtocolId(aProtoId)
-        , mTrans(nullptr)
-    {
-        MOZ_ASSERT(NS_IsMainThread() || AllowNonMainThreadUse());
-    }
-
+    explicit IToplevelProtocol(ProtocolId aProtoId);
     ~IToplevelProtocol();
 
     /**
      * Add an actor to the list of actors that have been opened by this
      * protocol.
      */
     void AddOpenedActor(IToplevelProtocol* aActor);
 
@@ -204,61 +202,43 @@ public:
     {
         mTrans = aTrans;
     }
 
     Transport* GetTransport() const { return mTrans; }
 
     ProtocolId GetProtocolId() const { return mProtocolId; }
 
-    /**
-     * Return first of actors of top level protocols opened by this one.
-     */
-    IToplevelProtocol* GetFirstOpenedActors()
-    {
-        MOZ_ASSERT(NS_IsMainThread() || AllowNonMainThreadUse());
-        return mOpenActors.getFirst();
-    }
-    const IToplevelProtocol* GetFirstOpenedActors() const
-    {
-        MOZ_ASSERT(NS_IsMainThread() || AllowNonMainThreadUse());
-        return mOpenActors.getFirst();
-    }
+    void GetOpenedActors(nsTArray<IToplevelProtocol*>& aActors);
+
+    // This Unsafe version should only be used when all other threads are
+    // frozen, since it performs no locking. It also takes a stack-allocated
+    // array and its size (number of elements) rather than an nsTArray. The Nuwa
+    // code that calls this function is not allowed to allocate memory.
+    size_t GetOpenedActorsUnsafe(IToplevelProtocol** aActors, size_t aActorsMax);
 
     virtual IToplevelProtocol*
     CloneToplevel(const InfallibleTArray<ProtocolFdMapping>& aFds,
                   base::ProcessHandle aPeerProcess,
                   ProtocolCloneContext* aCtx);
 
     void CloneOpenedToplevels(IToplevelProtocol* aTemplate,
                               const InfallibleTArray<ProtocolFdMapping>& aFds,
                               base::ProcessHandle aPeerProcess,
                               ProtocolCloneContext* aCtx);
 
-#ifdef MOZ_IPDL_TESTS
-    static void SetAllowNonMainThreadUse();
-#endif
+private:
+    void AddOpenedActorLocked(IToplevelProtocol* aActor);
+    void GetOpenedActorsLocked(nsTArray<IToplevelProtocol*>& aActors);
 
-    static bool AllowNonMainThreadUse() {
-#ifdef MOZ_IPDL_TESTS
-        return sAllowNonMainThreadUse;
-#else
-        return false;
-#endif
-    }
-
-private:
     LinkedList<IToplevelProtocol> mOpenActors; // All protocol actors opened by this.
+    IToplevelProtocol* mOpener;
 
     ProtocolId mProtocolId;
     Transport* mTrans;
-
-#ifdef MOZ_IPDL_TESTS
-    static bool sAllowNonMainThreadUse;
-#endif
 };
 
 
 inline bool
 LoggingEnabled()
 {
 #if defined(DEBUG)
     return !!PR_GetEnv("MOZ_IPC_MESSAGE_LOG");
--- a/ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp
+++ b/ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp
@@ -134,20 +134,16 @@ DeferredParentShutdown();
 void
 IPDLUnitTestThreadMain(char *testString);
 
 void
 IPDLUnitTestMain(void* aData)
 {
     char* testString = reinterpret_cast<char*>(aData);
 
-    // Some tests require this, and we don't care what thread we're on if we're
-    // not using Nuwa.
-    mozilla::ipc::IToplevelProtocol::SetAllowNonMainThreadUse();
-
     // Check if we are to run the test using threads instead:
     const char *prefix = "thread:";
     const int prefixLen = strlen(prefix);
     if (!strncmp(testString, prefix, prefixLen)) {
         IPDLUnitTestThreadMain(testString + prefixLen);
         return;
     }
 
@@ -373,20 +369,16 @@ DeleteChildActor()
     }
 }
 
 void
 IPDLUnitTestChildInit(IPC::Channel* transport,
                       base::ProcessHandle parent,
                       MessageLoop* worker)
 {
-    // Some tests require this, and we don't care what thread we're on if we're
-    // not using Nuwa.
-    mozilla::ipc::IToplevelProtocol::SetAllowNonMainThreadUse();
-
     switch (IPDLUnitTest()) {
 //-----------------------------------------------------------------------------
 //===== TEMPLATED =====
 ${CHILD_INIT_CASES}
 //-----------------------------------------------------------------------------
 
     default:
         fail("not reached");
--- a/testing/mochitest/mochitest_options.py
+++ b/testing/mochitest/mochitest_options.py
@@ -862,17 +862,17 @@ class B2GOptions(MochitestOptions):
         defaults["httpPort"] = DEFAULT_PORTS['http']
         defaults["sslPort"] = DEFAULT_PORTS['https']
         defaults["logFile"] = "mochitest.log"
         defaults["autorun"] = True
         defaults["closeWhenDone"] = True
         defaults["testPath"] = ""
         defaults["extensionsToExclude"] = ["specialpowers"]
         # See dependencies of bug 1038943.
-        defaults["defaultLeakThreshold"] = 5404
+        defaults["defaultLeakThreshold"] = 5536
         self.set_defaults(**defaults)
 
     def verifyRemoteOptions(self, options):
         if options.remoteWebServer is None:
             if os.name != "nt":
                 options.remoteWebServer = moznetwork.get_ip()
             else:
                 self.error(