Bug 1131463 - Report AtomicRefCounterWithFinalize doing the wrong thing with AddRef and Release in release build as well. r=sotaro

dom/media/omx/MediaCodecReader.cpp

@@ -768,7 +768,7 @@ void
 MediaCodecReader::TextureClientRecycleCallback(TextureClient* aClient)
 {
   MOZ_ASSERT(aClient, "aClient should not be nullptr in RecycleCallback()");
-
+  MOZ_ASSERT(!aClient->IsDead());
   size_t index = 0;
 
   {

dom/media/omx/OmxDecoder.cpp

@@ -915,6 +915,7 @@ void OmxDecoder::RecycleCallbackImp(TextureClient* aClient)
 /* static */ void
 OmxDecoder::RecycleCallback(TextureClient* aClient, void* aClosure)
 {
+  MOZ_ASSERT(aClient && !aClient->IsDead());
   OmxDecoder* decoder = static_cast<OmxDecoder*>(aClosure);
   decoder->RecycleCallbackImp(aClient);
 }

dom/media/platforms/gonk/GonkVideoDecoderManager.cpp

@@ -569,6 +569,7 @@ GonkVideoDecoderManager::GetColorConverterBuffer(int32_t aWidth, int32_t aHeight
 void
 GonkVideoDecoderManager::RecycleCallback(TextureClient* aClient, void* aClosure)
 {
+  MOZ_ASSERT(aClient && !aClient->IsDead());
   GonkVideoDecoderManager* videoManager = static_cast<GonkVideoDecoderManager*>(aClosure);
   GrallocTextureClientOGL* client = static_cast<GrallocTextureClientOGL*>(aClient);
   aClient->ClearRecycleCallback();

gfx/layers/AtomicRefCountedWithFinalize.h

@@ -11,6 +11,7 @@
 #include "MainThreadUtils.h"
 #include "base/message_loop.h"
 #include "base/task.h"
+#include "mozilla/gfx/Logging.h"
 
 namespace mozilla {
 
@@ -24,7 +25,11 @@ class AtomicRefCountedWithFinalize
       , mMessageLoopToPostDestructionTo(nullptr)
     {}
 
-    ~AtomicRefCountedWithFinalize() {}
+    ~AtomicRefCountedWithFinalize() {
+      if (mRefCount >= 0) {
+        gfxCriticalError() << "Deleting referenced object? " << mRefCount;
+      }
+    }
 
     void SetMessageLoopToPostDestructionTo(MessageLoop* l) {
       MOZ_ASSERT(NS_IsMainThread());
@@ -38,23 +43,33 @@ class AtomicRefCountedWithFinalize
 
   public:
     void AddRef() {
-      MOZ_ASSERT(mRefCount >= 0);
       ++mRefCount;
     }
 
     void Release() {
-      MOZ_ASSERT(mRefCount > 0);
       // Read mRecycleCallback early so that it does not get set to
-      // deleted memory, if the object is goes away.
+      // deleted memory, if the object is goes away.  See bug 994903.
+      // This saves us in the case where there is no callback, so that
+      // we can do the "else if" below.
       RecycleCallback recycleCallback = mRecycleCallback;
       int currCount = --mRefCount;
+      if (currCount < 0) {
+        gfxCriticalError() << "Invalid reference count release" << currCount;
+        ++mRefCount;
+        return;
+      }
+
       if (0 == currCount) {
+        mRefCount = detail::DEAD;
+        MOZ_ASSERT(IsDead());
+
         // Recycle listeners must call ClearRecycleCallback
         // before releasing their strong reference.
-        MOZ_ASSERT(mRecycleCallback == nullptr);
-#ifdef DEBUG
-        mRefCount = detail::DEAD;
-#endif
+        if (mRecycleCallback) {
+          gfxCriticalError() << "About to release with valid callback";
+          mRecycleCallback = nullptr;
+        }
+
         T* derived = static_cast<T*>(this);
         derived->Finalize();
         if (MOZ_LIKELY(!mMessageLoopToPostDestructionTo)) {
@@ -69,6 +84,10 @@ class AtomicRefCountedWithFinalize
           }
         }
       } else if (1 == currCount && recycleCallback) {
+        // There is nothing enforcing this in the code, except how the callers
+        // are being careful to never let the reference count go down if there
+        // is a callback.
+        MOZ_ASSERT(!IsDead());
         T* derived = static_cast<T*>(this);
         recycleCallback(derived, mClosure);
       }
@@ -81,16 +100,27 @@ class AtomicRefCountedWithFinalize
      */
     void SetRecycleCallback(RecycleCallback aCallback, void* aClosure)
     {
+      MOZ_ASSERT(!IsDead());
       mRecycleCallback = aCallback;
       mClosure = aClosure;
     }
-    void ClearRecycleCallback() { SetRecycleCallback(nullptr, nullptr); }
+    void ClearRecycleCallback()
+    {
+      MOZ_ASSERT(!IsDead());
+      SetRecycleCallback(nullptr, nullptr);
+    }
 
     bool HasRecycleCallback() const
     {
+      MOZ_ASSERT(!IsDead());
       return !!mRecycleCallback;
     }
 
+    bool IsDead() const
+    {
+      return mRefCount < 0;
+    }
+
 private:
     RecycleCallback mRecycleCallback;
     void *mClosure;

gfx/layers/client/TextureClientRecycleAllocator.cpp

@@ -235,6 +235,7 @@ TextureClientRecycleAllocatorImp::RecycleCallbackImp(TextureClient* aClient)
 /* static */ void
 TextureClientRecycleAllocatorImp::RecycleCallback(TextureClient* aClient, void* aClosure)
 {
+  MOZ_ASSERT(aClient && !aClient->IsDead());
   TextureClientRecycleAllocatorImp* recycleAllocator = static_cast<TextureClientRecycleAllocatorImp*>(aClosure);
   recycleAllocator->RecycleCallbackImp(aClient);
 }

gfx/layers/composite/TextureHost.cpp

@@ -764,7 +764,7 @@ TextureParent::~TextureParent()
   }
 }
 
-static void RecycleCallback(TextureHost* textureHost, void* aClosure) {
+static void RecycleCallback(TextureHost*, void* aClosure) {
   TextureParent* tp = reinterpret_cast<TextureParent*>(aClosure);
   tp->CompositorRecycle();
 }

mfbt/RefPtr.h

@@ -60,9 +60,7 @@ template<typename T> OutParamRef<T> byRef(RefPtr<T>&);
  * section of your class, where ClassName is the name of your class.
  */
 namespace detail {
-#ifdef DEBUG
 const MozRefCountType DEAD = 0xffffdead;
-#endif
 
 // When building code that gets compiled into Gecko, try to use the
 // trace-refcount leak logging facilities.

widget/gonk/nativewindow/GonkNativeWindowICS.cpp

@@ -499,6 +499,7 @@ GonkNativeWindow::RecycleCallback(TextureClient* client, void* closure) {
   GonkNativeWindow* nativeWindow =
     static_cast<GonkNativeWindow*>(closure);
 
+  MOZ_ASSERT(client && !client->IsDead());
   client->ClearRecycleCallback();
   nativeWindow->returnBuffer(client);
 }

widget/gonk/nativewindow/GonkNativeWindowJB.cpp

@@ -134,6 +134,7 @@ GonkNativeWindow::RecycleCallback(TextureClient* client, void* closure) {
   GonkNativeWindow* nativeWindow =
     static_cast<GonkNativeWindow*>(closure);
 
+  MOZ_ASSERT(client && !client->IsDead());
   client->ClearRecycleCallback();
   nativeWindow->returnBuffer(client);
 }

widget/gonk/nativewindow/GonkNativeWindowKK.cpp

@@ -134,6 +134,7 @@ GonkNativeWindow::RecycleCallback(TextureClient* client, void* closure) {
   GonkNativeWindow* nativeWindow =
     static_cast<GonkNativeWindow*>(closure);
 
+  MOZ_ASSERT(client && !client->IsDead());
   client->ClearRecycleCallback();
   nativeWindow->returnBuffer(client);
 }

widget/gonk/nativewindow/GonkNativeWindowLL.cpp

@@ -145,6 +145,7 @@ GonkNativeWindow::RecycleCallback(TextureClient* client, void* closure) {
     GonkNativeWindow* nativeWindow =
         static_cast<GonkNativeWindow*>(closure);
 
+    MOZ_ASSERT(client && !client->IsDead());
     client->ClearRecycleCallback();
     nativeWindow->returnBuffer(client);
 }