Bug 1313351 - Fix js::RefCounted to not do leak checking. r=jandem
authorLuke Wagner <luke@mozilla.com>
Fri, 23 Dec 2016 21:13:26 +0100
changeset 372242 47d317cfa2672e3e843da7a369072590460e96c6
parent 372241 1f3cc2963f093a5a3bdff31382f5c5fe6d24ee9f
child 372243 806029a2c31dc314b01911b22c75702597a160e9
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjandem
bugs1313351
milestone53.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 1313351 - Fix js::RefCounted to not do leak checking. r=jandem
js/public/RefCounted.h
js/src/jsapi.h
mfbt/RefCounted.h
--- a/js/public/RefCounted.h
+++ b/js/public/RefCounted.h
@@ -2,26 +2,90 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef js_RefCounted_h
 #define js_RefCounted_h
 
-#include "mozilla/RefCounted.h"
+#include "mozilla/Atomics.h"
+#include "mozilla/RefCountType.h"
 
 #include "js/Utility.h"
 
+// These types implement the same interface as mozilla::(Atomic)RefCounted and
+// must be used instead of mozilla::(Atomic)RefCounted for everything in
+// SpiderMonkey. There are two reasons:
+//  - Release() needs to call js_delete, not delete
+//  - SpiderMonkey does not have MOZILLA_INTERNAL_API defined which can lead
+//    to ODR violations that show up as spurious leak reports when ref-counted
+//    types are allocated by SpiderMonkey and released by Gecko (or vice versa).
+
 namespace js {
 
-// Replacement for mozilla::RefCounted and mozilla::external::AtomicRefCounted
-// that default to JS::DeletePolicy.
+template <typename T>
+class RefCounted
+{
+    static const MozRefCountType DEAD = 0xffffdead;
+
+  protected:
+    RefCounted() : mRefCnt(0) {}
+    ~RefCounted() { MOZ_ASSERT(mRefCnt == DEAD); }
+
+  public:
+    void AddRef() const
+    {
+        MOZ_ASSERT(int32_t(mRefCnt) >= 0);
+        ++mRefCnt;
+    }
+
+    void Release() const
+    {
+      MOZ_ASSERT(int32_t(mRefCnt) > 0);
+      MozRefCountType cnt = --mRefCnt;
+      if (0 == cnt) {
+#ifdef DEBUG
+          mRefCnt = DEAD;
+#endif
+          js_delete(const_cast<T*>(static_cast<const T*>(this)));
+      }
+    }
+
+  private:
+    mutable MozRefCountType mRefCnt;
+};
 
-template <typename T, typename D = JS::DeletePolicy<T>>
-using RefCounted = mozilla::RefCounted<T, D>;
+template <typename T>
+class AtomicRefCounted
+{
+    static const MozRefCountType DEAD = 0xffffdead;
+
+  protected:
+    AtomicRefCounted() : mRefCnt(0) {}
+    ~AtomicRefCounted() { MOZ_ASSERT(mRefCnt == DEAD); }
+
+  public:
+    void AddRef() const
+    {
+        MOZ_ASSERT(int32_t(mRefCnt) >= 0);
+        ++mRefCnt;
+    }
 
-template <typename T, typename D = JS::DeletePolicy<T>>
-using AtomicRefCounted = mozilla::external::AtomicRefCounted<T, D>;
+    void Release() const
+    {
+        MOZ_ASSERT(int32_t(mRefCnt) > 0);
+        MozRefCountType cnt = --mRefCnt;
+        if (0 == cnt) {
+#ifdef DEBUG
+            mRefCnt = DEAD;
+#endif
+            js_delete(const_cast<T*>(static_cast<const T*>(this)));
+        }
+    }
+
+  private:
+    mutable mozilla::Atomic<MozRefCountType> mRefCnt;
+};
 
 } // namespace js
 
 #endif /* js_RefCounted_h */
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -6103,17 +6103,16 @@ SetBuildIdOp(JSContext* cx, BuildIdOp bu
  * bytecode file descriptor and, if valid, the compiled-code file descriptor.
  * The JS::WasmObject is then transported to the JSRuntime thread (which
  * originated the request) and the wrapping WebAssembly.Module object is created
  * by calling createObject().
  */
 
 struct WasmModule : js::AtomicRefCounted<WasmModule>
 {
-    MOZ_DECLARE_REFCOUNTED_TYPENAME(WasmModule)
     virtual ~WasmModule() {}
 
     virtual void serializedSize(size_t* maybeBytecodeSize, size_t* maybeCompiledSize) const = 0;
     virtual void serialize(uint8_t* maybeBytecodeBegin, size_t maybeBytecodeSize,
                            uint8_t* maybeCompiledBegin, size_t maybeCompiledSize) const = 0;
 
     virtual JSObject* createObject(JSContext* cx) = 0;
 };
--- a/mfbt/RefCounted.h
+++ b/mfbt/RefCounted.h
@@ -11,17 +11,16 @@
 
 #include "mozilla/AlreadyAddRefed.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Atomics.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/Move.h"
 #include "mozilla/RefCountType.h"
 #include "mozilla/TypeTraits.h"
-#include "mozilla/UniquePtr.h"
 
 #if defined(MOZILLA_INTERNAL_API)
 #include "nsXPCOM.h"
 #endif
 
 #if defined(MOZILLA_INTERNAL_API) && \
     (defined(DEBUG) || defined(FORCE_BUILD_REFCNT_LOGGING))
 #define MOZ_REFCOUNTED_LEAK_CHECKING
@@ -48,16 +47,19 @@ namespace mozilla {
  * to 0, the RefCounted<T> "dies" and is destroyed.  The "destroyed"
  * state is represented in DEBUG builds by refcount==0xffffdead.  This
  * state distinguishes use-before-ref (refcount==0) from
  * use-after-destroy (refcount==0xffffdead).
  *
  * Note that when deriving from RefCounted or AtomicRefCounted, you
  * should add MOZ_DECLARE_REFCOUNTED_TYPENAME(ClassName) to the public
  * section of your class, where ClassName is the name of your class.
+ *
+ * Note: SpiderMonkey should use js::RefCounted instead since that type
+ * will use appropriate js_delete and also not break ref-count logging.
  */
 namespace detail {
 const MozRefCountType DEAD = 0xffffdead;
 
 // When building code that gets compiled into Gecko, try to use the
 // trace-refcount leak logging facilities.
 #ifdef MOZ_REFCOUNTED_LEAK_CHECKING
 class RefCountLogger
@@ -82,17 +84,17 @@ public:
 
 // This is used WeakPtr.h as well as this file.
 enum RefCountAtomicity
 {
   AtomicRefCount,
   NonAtomicRefCount
 };
 
-template<typename T, RefCountAtomicity Atomicity, typename D>
+template<typename T, RefCountAtomicity Atomicity>
 class RefCounted
 {
 protected:
   RefCounted() : mRefCnt(0) {}
   ~RefCounted() { MOZ_ASSERT(mRefCnt == detail::DEAD); }
 
 public:
   // Compatibility with nsRefPtr.
@@ -128,17 +130,17 @@ public:
     if (0 == cnt) {
       // Because we have atomically decremented the refcount above, only
       // one thread can get a 0 count here, so as long as we can assume that
       // everything else in the system is accessing this object through
       // RefPtrs, it's safe to access |this| here.
 #ifdef DEBUG
       mRefCnt = detail::DEAD;
 #endif
-      D()(const_cast<T*>(static_cast<const T*>(this)));
+      delete static_cast<const T*>(this);
     }
   }
 
   // Compatibility with wtf::RefPtr.
   void ref() { AddRef(); }
   void deref() { Release(); }
   MozRefCountType refCount() const { return mRefCnt; }
   bool hasOneRef() const
@@ -167,18 +169,18 @@ private:
 // two small inline functions which will hopefully get eliminated by the linker
 // in non-leak-checking builds.
 #define MOZ_DECLARE_REFCOUNTED_TYPENAME(T) \
   const char* typeName() const { return #T; } \
   size_t typeSize() const { return sizeof(*this); }
 
 } // namespace detail
 
-template<typename T, class D = DefaultDelete<T>>
-class RefCounted : public detail::RefCounted<T, detail::NonAtomicRefCount, D>
+template<typename T>
+class RefCounted : public detail::RefCounted<T, detail::NonAtomicRefCount>
 {
 public:
   ~RefCounted()
   {
     static_assert(IsBaseOf<RefCounted, T>::value,
                   "T must derive from RefCounted<T>");
   }
 };
@@ -187,19 +189,19 @@ namespace external {
 
 /**
  * AtomicRefCounted<T> is like RefCounted<T>, with an atomically updated
  * reference counter.
  *
  * NOTE: Please do not use this class, use NS_INLINE_DECL_THREADSAFE_REFCOUNTING
  * instead.
  */
-template<typename T, typename D = DefaultDelete<T>>
+template<typename T>
 class AtomicRefCounted :
-  public mozilla::detail::RefCounted<T, mozilla::detail::AtomicRefCount, D>
+  public mozilla::detail::RefCounted<T, mozilla::detail::AtomicRefCount>
 {
 public:
   ~AtomicRefCounted()
   {
     static_assert(IsBaseOf<AtomicRefCounted, T>::value,
                   "T must derive from AtomicRefCounted<T>");
   }
 };