Bug 834459 - Specialize RefCounted template for LibHandle to better allow refcounting during destructor execution. r=nfroyd
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 29 Jan 2013 09:35:32 +0100
changeset 120171 4631eeae9ff4505721dffb0fc4f65b395e4a73ae
parent 120170 bc2bbe9836c7c40eedca35e79d4f73974736e20d
child 120172 b02f1d2eafb73d670905a3cc5d521a08110e2d3f
push id24239
push userryanvm@gmail.com
push dateTue, 29 Jan 2013 14:36:31 +0000
treeherdermozilla-central@d3a5e1de98b0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnfroyd
bugs834459
milestone21.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 834459 - Specialize RefCounted template for LibHandle to better allow refcounting during destructor execution. r=nfroyd
mozglue/linker/CustomElf.cpp
mozglue/linker/ElfLoader.h
--- a/mozglue/linker/CustomElf.cpp
+++ b/mozglue/linker/CustomElf.cpp
@@ -221,20 +221,16 @@ CustomElf::Load(Mappable *mappable, cons
   elf->stats("oneLibLoaded");
   debug("CustomElf::Load(\"%s\", 0x%x) = %p", path, flags,
         static_cast<void *>(elf));
   return elf;
 }
 
 CustomElf::~CustomElf()
 {
-  /* While running the destructors, on-demand decompression may get new
-   * references on this object, and releasing these references would call
-   * the destructor again. Avoid this by always having the refcount > 0 */
-  AddRef();
   debug("CustomElf::~CustomElf(%p [\"%s\"])",
         reinterpret_cast<void *>(this), GetPath());
   CallFini();
   /* Normally, __cxa_finalize is called by the .fini function. However,
    * Android NDK before r6b doesn't do that. Our wrapped cxa_finalize only
    * calls destructors once, so call it in all cases. */
   ElfLoader::__wrap_cxa_finalize(this);
   delete mappable;
--- a/mozglue/linker/ElfLoader.h
+++ b/mozglue/linker/ElfLoader.h
@@ -42,16 +42,35 @@ extern "C" {
     Elf::Half dlpi_phnum;
   };
 
   typedef int (*dl_phdr_cb)(struct dl_phdr_info *, size_t, void *);
   int __wrap_dl_iterate_phdr(dl_phdr_cb callback, void *data);
 }
 
 /**
+ * Specialize RefCounted template for LibHandle. We may get references to
+ * LibHandles during the execution of their destructor, so we need
+ * RefCounted<LibHandle>::Release to support some reentrancy. See further
+ * below.
+ */
+class LibHandle;
+
+namespace mozilla {
+
+template <> inline void RefCounted<LibHandle>::Release();
+
+template <> inline RefCounted<LibHandle>::~RefCounted()
+{
+  MOZ_ASSERT(refCnt == 0x7fffdead);
+}
+
+} /* namespace mozilla */
+
+/**
  * Abstract class for loaded libraries. Libraries may be loaded through the
  * system linker or this linker, both cases will be derived from this class.
  */
 class LibHandle: public mozilla::RefCounted<LibHandle>
 {
 public:
   /**
    * Constructor. Takes the path of the loaded library and will store a copy
@@ -139,16 +158,46 @@ protected:
   virtual bool IsSystemElf() const { return false; }
 
 private:
   int directRefCnt;
   char *path;
 };
 
 /**
+ * Specialized RefCounted<LibHandle>::Release. Under normal operation, when
+ * refCnt reaches 0, the LibHandle is deleted. Its refCnt is however increased
+ * to 1 on normal builds, and 0x7fffdead on debug builds so that the LibHandle
+ * can still be referenced while the destructor is executing. The refCnt is
+ * allowed to grow > 0x7fffdead, but not to decrease under that value, which
+ * would mean too many Releases from within the destructor.
+ */
+namespace mozilla {
+
+template <> inline void RefCounted<LibHandle>::Release() {
+#ifdef DEBUG
+  if (refCnt > 0x7fff0000)
+    MOZ_ASSERT(refCnt > 0x7fffdead);
+#endif
+  MOZ_ASSERT(refCnt > 0);
+  if (refCnt > 0) {
+    if (0 == --refCnt) {
+#ifdef DEBUG
+      refCnt = 0x7fffdead;
+#else
+      refCnt = 1;
+#endif
+      delete static_cast<LibHandle*>(this);
+    }
+  }
+}
+
+} /* namespace mozilla */
+
+/**
  * Class handling libraries loaded by the system linker
  */
 class SystemElf: public LibHandle
 {
 public:
   /**
    * Returns a new SystemElf for the given path. The given flags are passed
    * to dlopen().