Bug 1121676 - Use a lock to protect the list of top-level actors (r=bent)

dom/ipc/ContentChild.cpp

@@ -551,19 +551,20 @@ InitOnContentProcessCreated()
 
 #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();
+  nsTArray<IToplevelProtocol*> actors;
-  while (toplevel != nullptr) {
+  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
@@ -2677,9 +2678,10 @@ GetProtoFdInfos(NuwaProtoFdInfo* aInfoList,
         content->GetTransport()->GetFileDescriptor();
     i++;
 
-    for (IToplevelProtocol* actor = content->GetFirstOpenedActors();
+    IToplevelProtocol* actors[NUWA_TOPLEVEL_MAX];
-         actor != nullptr;
+    size_t count = content->GetOpenedActorsUnsafe(actors, ArrayLength(actors));
-         actor = actor->getNext()) {
+    for (size_t j = 0; j < count; j++) {
+        IToplevelProtocol* actor = actors[j];
         if (i >= aInfoListSize) {
             NS_RUNTIMEABORT("Too many top level protocols!");
         }

ipc/glue/ProtocolUtils.cpp

@@ -21,19 +21,56 @@ using base::ProcessId;
 namespace mozilla {
 namespace ipc {
 
-#ifdef MOZ_IPDL_TESTS
+static Atomic<size_t> gNumProtocols;
-bool IToplevelProtocol::sAllowNonMainThreadUse;
+static StaticAutoPtr<Mutex> gProtocolMutex;
-#endif
+
+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());
+  bool last = false;
-  mOpenActors.clear();
+
+  {
+    MutexAutoLock al(*gProtocolMutex);
+
+    for (IToplevelProtocol* actor = mOpenActors.getFirst();
+         actor;
+         actor = actor->getNext()) {
+      actor->mOpener = nullptr;
     }
 
-void IToplevelProtocol::AddOpenedActor(IToplevelProtocol* aActor)
+    mOpenActors.clear();
+
+    if (mOpener) {
+      removeFrom(mOpener->mOpenActors);
+    }
+
+    gNumProtocols--;
+    last = gNumProtocols == 0;
+  }
+
+  if (last) {
+    gProtocolMutex = nullptr;
+  }
+}
+
+void
+IToplevelProtocol::AddOpenedActorLocked(IToplevelProtocol* aActor)
 {
-  MOZ_ASSERT(NS_IsMainThread() || AllowNonMainThreadUse());
+  gProtocolMutex->AssertCurrentThreadOwns();
 
 #ifdef DEBUG
   for (const IToplevelProtocol* actor = mOpenActors.getFirst();
@@ -44,9 +81,49 @@ void IToplevelProtocol::AddOpenedActor(IToplevelProtocol* aActor)
   }
 #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,
@@ -62,21 +139,16 @@ IToplevelProtocol::CloneOpenedToplevels(IToplevelProtocol* aTemplate,
                                         base::ProcessHandle aPeerProcess,
                                         ProtocolCloneContext* aCtx)
 {
-  for (IToplevelProtocol* actor = aTemplate->GetFirstOpenedActors();
+  MutexAutoLock al(*gProtocolMutex);
-       actor;
-       actor = actor->getNext()) {
-    IToplevelProtocol* newactor = actor->CloneToplevel(aFds, aPeerProcess, aCtx);
-    AddOpenedActor(newactor);
-  }
-}
 
-#ifdef MOZ_IPDL_TESTS
+  nsTArray<IToplevelProtocol*> actors;
-void
+  aTemplate->GetOpenedActorsLocked(actors);
-IToplevelProtocol::SetAllowNonMainThreadUse()
+
-{
+  for (size_t i = 0; i < actors.Length(); i++) {
-  sAllowNonMainThreadUse = true;
+    IToplevelProtocol* newactor = actors[i]->CloneToplevel(aFds, aPeerProcess, aCtx);
+    AddOpenedActorLocked(newactor);
+  }
 }
-#endif
 
 class ChannelOpened : public IPC::Message
 {

ipc/glue/ProtocolUtils.h

@@ -21,6 +21,7 @@
 #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)
@@ -181,16 +182,13 @@ public:
  * IToplevelProtocol tracks all top-level protocol actors created from
  * this protocol actor.
  */
-class IToplevelProtocol : public LinkedListElement<IToplevelProtocol>
+class IToplevelProtocol : private LinkedListElement<IToplevelProtocol>
 {
-protected:
+    friend class LinkedList<IToplevelProtocol>;
-    explicit IToplevelProtocol(ProtocolId aProtoId)
+    friend class LinkedListElement<IToplevelProtocol>;
-        : mProtocolId(aProtoId)
-        , mTrans(nullptr)
-    {
-        MOZ_ASSERT(NS_IsMainThread() || AllowNonMainThreadUse());
-    }
 
+protected:
+    explicit IToplevelProtocol(ProtocolId aProtoId);
     ~IToplevelProtocol();
 
     /**
@@ -209,19 +207,13 @@ public:
 
     ProtocolId GetProtocolId() const { return mProtocolId; }
 
-    /**
+    void GetOpenedActors(nsTArray<IToplevelProtocol*>& aActors);
-     * Return first of actors of top level protocols opened by this one.
+
-     */
+    // This Unsafe version should only be used when all other threads are
-    IToplevelProtocol* GetFirstOpenedActors()
+    // 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
-        MOZ_ASSERT(NS_IsMainThread() || AllowNonMainThreadUse());
+    // code that calls this function is not allowed to allocate memory.
-        return mOpenActors.getFirst();
+    size_t GetOpenedActorsUnsafe(IToplevelProtocol** aActors, size_t aActorsMax);
-    }
-    const IToplevelProtocol* GetFirstOpenedActors() const
-    {
-        MOZ_ASSERT(NS_IsMainThread() || AllowNonMainThreadUse());
-        return mOpenActors.getFirst();
-    }
 
     virtual IToplevelProtocol*
     CloneToplevel(const InfallibleTArray<ProtocolFdMapping>& aFds,
@@ -233,27 +225,15 @@ public:
                               base::ProcessHandle aPeerProcess,
                               ProtocolCloneContext* aCtx);
 
-#ifdef MOZ_IPDL_TESTS
-    static void SetAllowNonMainThreadUse();
-#endif
-
-    static bool AllowNonMainThreadUse() {
-#ifdef MOZ_IPDL_TESTS
-        return sAllowNonMainThreadUse;
-#else
-        return false;
-#endif
-    }
-
 private:
+    void AddOpenedActorLocked(IToplevelProtocol* aActor);
+    void GetOpenedActorsLocked(nsTArray<IToplevelProtocol*>& aActors);
+
     LinkedList<IToplevelProtocol> mOpenActors; // All protocol actors opened by this.
+    IToplevelProtocol* mOpener;
 
     ProtocolId mProtocolId;
     Transport* mTrans;
-
-#ifdef MOZ_IPDL_TESTS
-    static bool sAllowNonMainThreadUse;
-#endif
 };
 
 

ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp

@@ -139,10 +139,6 @@ 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);
@@ -378,10 +374,6 @@ 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 =====

testing/mochitest/mochitest_options.py

@@ -867,7 +867,7 @@ class B2GOptions(MochitestOptions):
         defaults["testPath"] = ""
         defaults["extensionsToExclude"] = ["specialpowers"]
         # See dependencies of bug 1038943.
-        defaults["defaultLeakThreshold"] = 5404
+        defaults["defaultLeakThreshold"] = 5536
         self.set_defaults(**defaults)
 
     def verifyRemoteOptions(self, options):