Bug 1461326 Add some release assertions to dom/clients/manager code. r=baku
authorBen Kelly <ben@wanderview.com>
Tue, 15 May 2018 08:53:54 -0700
changeset 418389 205e5ff205d3b9cc925ff932e09df7a76b6cf3fd
parent 418388 a256429fb18c0ed854124cf268c9caa90a75746f
child 418390 8a820882e9273ad19422b696de0c137acfadd5d5
push id33999
push userdluca@mozilla.com
push dateTue, 15 May 2018 21:54:51 +0000
treeherdermozilla-central@380cf87c1ee3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1461326
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1461326 Add some release assertions to dom/clients/manager code. r=baku
dom/clients/manager/ClientHandle.cpp
dom/clients/manager/ClientHandleOpChild.cpp
dom/clients/manager/ClientManager.cpp
dom/clients/manager/ClientThing.h
--- a/dom/clients/manager/ClientHandle.cpp
+++ b/dom/clients/manager/ClientHandle.cpp
@@ -44,24 +44,26 @@ ClientHandle::StartOp(const ClientOpCons
 {
   // Hold a ref to the client until the remote operation completes.  Otherwise
   // the ClientHandle might get de-refed and teardown the actor before we
   // get an answer.
   RefPtr<ClientHandle> kungFuGrip = this;
 
   MaybeExecute([aArgs, kungFuGrip, aRejectCallback,
                 resolve = Move(aResolveCallback)] (ClientHandleChild* aActor) {
+    MOZ_RELEASE_ASSERT(aActor);
     ClientHandleOpChild* actor =
       new ClientHandleOpChild(kungFuGrip, aArgs, Move(resolve),
                               Move(aRejectCallback));
     if (!aActor->SendPClientHandleOpConstructor(actor, aArgs)) {
       // Constructor failure will call reject callback via ActorDestroy()
       return;
     }
   }, [aRejectCallback] {
+    MOZ_RELEASE_ASSERT(aRejectCallback);
     aRejectCallback(NS_ERROR_DOM_INVALID_STATE_ERR);
   });
 }
 
 void
 ClientHandle::OnShutdownThing()
 {
   NS_ASSERT_OWNINGTHREAD(ClientHandle);
--- a/dom/clients/manager/ClientHandleOpChild.cpp
+++ b/dom/clients/manager/ClientHandleOpChild.cpp
@@ -34,15 +34,15 @@ ClientHandleOpChild::Recv__delete__(cons
 ClientHandleOpChild::ClientHandleOpChild(ClientHandle* aClientHandle,
                                          const ClientOpConstructorArgs& aArgs,
                                          const ClientOpCallback&& aResolveCallback,
                                          const ClientOpCallback&& aRejectCallback)
   : mClientHandle(aClientHandle)
   , mResolveCallback(Move(aResolveCallback))
   , mRejectCallback(Move(aRejectCallback))
 {
-  MOZ_DIAGNOSTIC_ASSERT(mClientHandle);
-  MOZ_DIAGNOSTIC_ASSERT(mResolveCallback);
-  MOZ_DIAGNOSTIC_ASSERT(mRejectCallback);
+  MOZ_RELEASE_ASSERT(mClientHandle);
+  MOZ_RELEASE_ASSERT(mResolveCallback);
+  MOZ_RELEASE_ASSERT(mRejectCallback);
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/clients/manager/ClientManager.cpp
+++ b/dom/clients/manager/ClientManager.cpp
@@ -22,18 +22,23 @@ namespace mozilla {
 namespace dom {
 
 using mozilla::ipc::BackgroundChild;
 using mozilla::ipc::PBackgroundChild;
 using mozilla::ipc::PrincipalInfo;
 
 namespace {
 
-uint32_t kBadThreadLocalIndex = -1;
+const uint32_t kBadThreadLocalIndex = -1;
+const uint32_t kThreadLocalMagic1 = 0x8d57eea6;
+const uint32_t kThreadLocalMagic2 = 0x59f375c9;
+uint32_t sClientManagerThreadLocalMagic1 = kThreadLocalMagic1;
 uint32_t sClientManagerThreadLocalIndex = kBadThreadLocalIndex;
+uint32_t sClientManagerThreadLocalMagic2 = kThreadLocalMagic2;
+uint32_t sClientManagerThreadLocalIndexDuplicate = kBadThreadLocalIndex;
 
 } // anonymous namespace
 
 ClientManager::ClientManager()
 {
   PBackgroundChild* parentActor = BackgroundChild::GetOrCreateForCurrentThread();
   if (NS_WARN_IF(!parentActor)) {
     Shutdown();
@@ -74,17 +79,21 @@ ClientManager::ClientManager()
 }
 
 ClientManager::~ClientManager()
 {
   NS_ASSERT_OWNINGTHREAD(ClientManager);
 
   Shutdown();
 
-  MOZ_DIAGNOSTIC_ASSERT(this == PR_GetThreadPrivate(sClientManagerThreadLocalIndex));
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalMagic1 == kThreadLocalMagic1);
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalMagic2 == kThreadLocalMagic2);
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalIndex != kBadThreadLocalIndex);
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalIndex == sClientManagerThreadLocalIndexDuplicate);
+  MOZ_RELEASE_ASSERT(this == PR_GetThreadPrivate(sClientManagerThreadLocalIndex));
 
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
   PRStatus status =
 #endif
     PR_SetThreadPrivate(sClientManagerThreadLocalIndex, nullptr);
   MOZ_DIAGNOSTIC_ASSERT(status == PR_SUCCESS);
 }
 
@@ -180,53 +189,65 @@ ClientManager::StartOp(const ClientOpCon
   RefPtr<ClientOpPromise> ref = promise.get();
   return ref.forget();
 }
 
 // static
 already_AddRefed<ClientManager>
 ClientManager::GetOrCreateForCurrentThread()
 {
-  MOZ_DIAGNOSTIC_ASSERT(sClientManagerThreadLocalIndex != kBadThreadLocalIndex);
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalMagic1 == kThreadLocalMagic1);
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalMagic2 == kThreadLocalMagic2);
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalIndex != kBadThreadLocalIndex);
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalIndex == sClientManagerThreadLocalIndexDuplicate);
   RefPtr<ClientManager> cm =
     static_cast<ClientManager*>(PR_GetThreadPrivate(sClientManagerThreadLocalIndex));
 
   if (!cm) {
     cm = new ClientManager();
 
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
     PRStatus status =
 #endif
       PR_SetThreadPrivate(sClientManagerThreadLocalIndex, cm.get());
     MOZ_DIAGNOSTIC_ASSERT(status == PR_SUCCESS);
   }
 
-  MOZ_ASSERT(cm);
+  MOZ_RELEASE_ASSERT(cm);
   return cm.forget();
 }
 
 WorkerPrivate*
 ClientManager::GetWorkerPrivate() const
 {
   NS_ASSERT_OWNINGTHREAD(ClientManager);
   MOZ_DIAGNOSTIC_ASSERT(GetActor());
   return GetActor()->GetWorkerPrivate();
 }
 
 // static
 void
 ClientManager::Startup()
 {
   MOZ_ASSERT(NS_IsMainThread());
+
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalMagic1 == kThreadLocalMagic1);
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalMagic2 == kThreadLocalMagic2);
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalIndex == kBadThreadLocalIndex);
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalIndex == sClientManagerThreadLocalIndexDuplicate);
+
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
   PRStatus status =
 #endif
     PR_NewThreadPrivateIndex(&sClientManagerThreadLocalIndex, nullptr);
   MOZ_DIAGNOSTIC_ASSERT(status == PR_SUCCESS);
 
+  MOZ_RELEASE_ASSERT(sClientManagerThreadLocalIndex != kBadThreadLocalIndex);
+  sClientManagerThreadLocalIndexDuplicate = sClientManagerThreadLocalIndex;
+
   ClientPrefsInit();
 }
 
 // static
 UniquePtr<ClientSource>
 ClientManager::CreateSource(ClientType aType, nsISerialEventTarget* aEventTarget,
                             nsIPrincipal* aPrincipal)
 {
--- a/dom/clients/manager/ClientThing.h
+++ b/dom/clients/manager/ClientThing.h
@@ -12,77 +12,99 @@ namespace mozilla {
 namespace dom {
 
 // Base class representing various Client "things" such as ClientHandle,
 // ClientSource, and ClientManager.  Currently it provides a common set
 // of code for handling activation and shutdown of IPC actors.
 template <typename ActorType>
 class ClientThing
 {
+  static const uint32_t kMagic1 = 0xC9FE2C9C;
+  static const uint32_t kMagic2 = 0x832072D4;
+
   ActorType* mActor;
+  uint32_t mMagic1;
+  uint32_t mMagic2;
   bool mShutdown;
 
 protected:
   ClientThing()
     : mActor(nullptr)
+    , mMagic1(kMagic1)
+    , mMagic2(kMagic2)
     , mShutdown(false)
   {
   }
 
   ~ClientThing()
   {
+    AssertIsValid();
     ShutdownThing();
+    mMagic1 = 0;
+    mMagic2 = 0;
+  }
+
+  void
+  AssertIsValid() const
+  {
+    MOZ_RELEASE_ASSERT(mMagic1 == kMagic1);
+    MOZ_RELEASE_ASSERT(mMagic2 == kMagic2);
   }
 
   // Return the current actor.
   ActorType*
   GetActor() const
   {
+    AssertIsValid();
     return mActor;
   }
 
   // Returns true if ShutdownThing() has been called.
   bool
   IsShutdown() const
   {
+    AssertIsValid();
     return mShutdown;
   }
 
   // Conditionally execute the given callable based on the current state.
   template<typename Callable>
   void
   MaybeExecute(const Callable& aSuccess,
                const std::function<void()>& aFailure = []{})
   {
+    AssertIsValid();
     if (mShutdown) {
       aFailure();
       return;
     }
     MOZ_DIAGNOSTIC_ASSERT(mActor);
     aSuccess(mActor);
   }
 
   // Attach activate the thing by attaching its underlying IPC actor.  This
   // will make the thing register as the actor's owner as well.  The actor
   // must call RevokeActor() to clear this weak back reference before its
   // destroyed.
   void
   ActivateThing(ActorType* aActor)
   {
+    AssertIsValid();
     MOZ_DIAGNOSTIC_ASSERT(aActor);
     MOZ_DIAGNOSTIC_ASSERT(!mActor);
     MOZ_DIAGNOSTIC_ASSERT(!mShutdown);
     mActor = aActor;
     mActor->SetOwner(this);
   }
 
   // Start destroying the underlying actor and disconnect the thing.
   void
   ShutdownThing()
   {
+    AssertIsValid();
     if (mShutdown) {
       return;
     }
     mShutdown = true;
 
     // If we are shutdown before the actor, then clear the weak references
     // between the actor and the thing.
     if (mActor) {
@@ -101,16 +123,17 @@ protected:
     // by default do nothing
   }
 
 public:
   // Clear the weak references between the thing and its IPC actor.
   void
   RevokeActor(ActorType* aActor)
   {
+    AssertIsValid();
     MOZ_DIAGNOSTIC_ASSERT(mActor);
     MOZ_DIAGNOSTIC_ASSERT(mActor == aActor);
     mActor->RevokeOwner(this);
     mActor = nullptr;
 
     // Also consider the ClientThing shutdown.  We simply set the flag
     // instead of calling ShutdownThing() to avoid calling MaybeStartTeardown()
     // on the destroyed actor.