Bug 1121297 (Part 2) - Make VolatileBuffer threadsafe. r=glandium
☠☠ backed out by a7b0fe4de103 ☠ ☠
authorSeth Fowler <seth@mozilla.com>
Fri, 16 Jan 2015 14:20:14 -0800
changeset 253110 28960bb167ef6156f2f3964d7e1f57eaf207bde4
parent 253109 005cf05954e7d3ecdccbb52d3385da937c5cbef2
child 253111 dae4958ba3bf884bd63013fea09074aa47eaa165
push id721
push userjlund@mozilla.com
push dateTue, 21 Apr 2015 23:03:33 +0000
treeherdermozilla-release@d27c9211ebb3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs1121297
milestone38.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 1121297 (Part 2) - Make VolatileBuffer threadsafe. r=glandium
memory/volatile/VolatileBuffer.h
memory/volatile/VolatileBufferAshmem.cpp
memory/volatile/VolatileBufferFallback.cpp
memory/volatile/VolatileBufferOSX.cpp
memory/volatile/VolatileBufferWindows.cpp
--- a/memory/volatile/VolatileBuffer.h
+++ b/memory/volatile/VolatileBuffer.h
@@ -1,16 +1,17 @@
 /* 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 mozalloc_VolatileBuffer_h
 #define mozalloc_VolatileBuffer_h
 
 #include "mozilla/mozalloc.h"
+#include "mozilla/Mutex.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/MemoryReporting.h"
 
 /* VolatileBuffer
  *
  * This class represents a piece of memory that can potentially be reclaimed
  * by the OS when not in use. As long as there are one or more
  * VolatileBufferPtrs holding on to a VolatileBuffer, the memory will remain
@@ -31,42 +32,51 @@
  * The OS cannot purge a buffer immediately after a VolatileBuffer is
  * initialized. At least one VolatileBufferPtr must be created before the
  * buffer can be purged, so the first use of VolatileBufferPtr does not need
  * to check WasPurged().
  *
  * When a buffer is purged, some or all of the buffer is zeroed out. This
  * API cannot tell which parts of the buffer were lost.
  *
- * VolatileBuffer is not thread safe. Do not use VolatileBufferPtrs on
- * different threads.
+ * VolatileBuffer and VolatileBufferPtr are threadsafe.
  */
 
 namespace mozilla {
 
-class VolatileBuffer : public RefCounted<VolatileBuffer>
+class VolatileBuffer
 {
   friend class VolatileBufferPtr_base;
 public:
   MOZ_DECLARE_REFCOUNTED_TYPENAME(VolatileBuffer)
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(VolatileBuffer)
+
   VolatileBuffer();
-  ~VolatileBuffer();
 
   /* aAlignment must be a multiple of the pointer size */
   bool Init(size_t aSize, size_t aAlignment = sizeof(void*));
 
   size_t HeapSizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const;
   size_t NonHeapSizeOfExcludingThis() const;
   bool OnHeap() const;
 
 protected:
   bool Lock(void** aBuf);
   void Unlock();
 
 private:
+  ~VolatileBuffer();
+
+  /**
+   * Protects mLockCount, mFirstLock, and changes to the volatility of our
+   * buffer.  Other member variables are read-only except in Init() and the
+   * destructor.
+   */
+  Mutex mMutex;
+
   void* mBuf;
   size_t mSize;
   int mLockCount;
 #if defined(ANDROID)
   int mFd;
 #elif defined(XP_DARWIN)
   bool mHeap;
 #elif defined(XP_WIN)
--- a/memory/volatile/VolatileBufferAshmem.cpp
+++ b/memory/volatile/VolatileBufferAshmem.cpp
@@ -17,17 +17,18 @@
 extern "C" int posix_memalign(void** memptr, size_t alignment, size_t size);
 #endif
 
 #define MIN_VOLATILE_ALLOC_SIZE 8192
 
 namespace mozilla {
 
 VolatileBuffer::VolatileBuffer()
-  : mBuf(nullptr)
+  : mMutex("VolatileBuffer")
+  , mBuf(nullptr)
   , mSize(0)
   , mLockCount(0)
   , mFd(-1)
 {
 }
 
 bool
 VolatileBuffer::Init(size_t aSize, size_t aAlignment)
@@ -67,42 +68,48 @@ heap_alloc:
 #else
   mBuf = memalign(aAlignment, aSize);
 #endif
   return !!mBuf;
 }
 
 VolatileBuffer::~VolatileBuffer()
 {
+  MOZ_ASSERT(mLockCount == 0, "Being destroyed with non-zero lock count?");
+
   if (OnHeap()) {
     free(mBuf);
   } else {
     munmap(mBuf, mSize);
     close(mFd);
   }
 }
 
 bool
 VolatileBuffer::Lock(void** aBuf)
 {
+  MutexAutoLock lock(mMutex);
+
   MOZ_ASSERT(mBuf, "Attempting to lock an uninitialized VolatileBuffer");
 
   *aBuf = mBuf;
   if (++mLockCount > 1 || OnHeap()) {
     return true;
   }
 
   // Zero offset and zero length means we want to pin/unpin the entire thing.
   struct ashmem_pin pin = { 0, 0 };
   return ioctl(mFd, ASHMEM_PIN, &pin) == ASHMEM_NOT_PURGED;
 }
 
 void
 VolatileBuffer::Unlock()
 {
+  MutexAutoLock lock(mMutex);
+
   MOZ_ASSERT(mLockCount > 0, "VolatileBuffer unlocked too many times!");
   if (--mLockCount || OnHeap()) {
     return;
   }
 
   struct ashmem_pin pin = { 0, 0 };
   ioctl(mFd, ASHMEM_UNPIN, &pin);
 }
--- a/memory/volatile/VolatileBufferFallback.cpp
+++ b/memory/volatile/VolatileBufferFallback.cpp
@@ -8,17 +8,18 @@
 
 #ifdef MOZ_MEMORY
 int posix_memalign(void** memptr, size_t alignment, size_t size);
 #endif
 
 namespace mozilla {
 
 VolatileBuffer::VolatileBuffer()
-  : mBuf(nullptr)
+  : mMutex("VolatileBuffer")
+  , mBuf(nullptr)
   , mSize(0)
   , mLockCount(0)
 {
 }
 
 bool VolatileBuffer::Init(size_t aSize, size_t aAlignment)
 {
   MOZ_ASSERT(!mSize && !mBuf, "Init called twice");
@@ -37,33 +38,39 @@ bool VolatileBuffer::Init(size_t aSize, 
 #else
 #error "No memalign implementation found"
 #endif
   return !!mBuf;
 }
 
 VolatileBuffer::~VolatileBuffer()
 {
+  MOZ_ASSERT(mLockCount == 0, "Being destroyed with non-zero lock count?");
+
   free(mBuf);
 }
 
 bool
 VolatileBuffer::Lock(void** aBuf)
 {
+  MutexAutoLock lock(mMutex);
+
   MOZ_ASSERT(mBuf, "Attempting to lock an uninitialized VolatileBuffer");
 
   *aBuf = mBuf;
   mLockCount++;
 
   return true;
 }
 
 void
 VolatileBuffer::Unlock()
 {
+  MutexAutoLock lock(mMutex);
+
   mLockCount--;
   MOZ_ASSERT(mLockCount >= 0, "VolatileBuffer unlocked too many times!");
 }
 
 bool
 VolatileBuffer::OnHeap() const
 {
   return true;
--- a/memory/volatile/VolatileBufferOSX.cpp
+++ b/memory/volatile/VolatileBufferOSX.cpp
@@ -11,17 +11,18 @@
 #include <sys/mman.h>
 #include <unistd.h>
 
 #define MIN_VOLATILE_ALLOC_SIZE 8192
 
 namespace mozilla {
 
 VolatileBuffer::VolatileBuffer()
-  : mBuf(nullptr)
+  : mMutex("VolatileBuffer")
+  , mBuf(nullptr)
   , mSize(0)
   , mLockCount(0)
   , mHeap(false)
 {
 }
 
 bool
 VolatileBuffer::Init(size_t aSize, size_t aAlignment)
@@ -48,26 +49,30 @@ VolatileBuffer::Init(size_t aSize, size_
 heap_alloc:
   (void)moz_posix_memalign(&mBuf, aAlignment, aSize);
   mHeap = true;
   return !!mBuf;
 }
 
 VolatileBuffer::~VolatileBuffer()
 {
+  MOZ_ASSERT(mLockCount == 0, "Being destroyed with non-zero lock count?");
+
   if (OnHeap()) {
     free(mBuf);
   } else {
     vm_deallocate(mach_task_self(), (vm_address_t)mBuf, mSize);
   }
 }
 
 bool
 VolatileBuffer::Lock(void** aBuf)
 {
+  MutexAutoLock lock(mMutex);
+
   MOZ_ASSERT(mBuf, "Attempting to lock an uninitialized VolatileBuffer");
 
   *aBuf = mBuf;
   if (++mLockCount > 1 || OnHeap()) {
     return true;
   }
 
   int state = VM_PURGABLE_NONVOLATILE;
@@ -77,16 +82,18 @@ VolatileBuffer::Lock(void** aBuf)
                         VM_PURGABLE_SET_STATE,
                         &state);
   return ret == KERN_SUCCESS && !(state & VM_PURGABLE_EMPTY);
 }
 
 void
 VolatileBuffer::Unlock()
 {
+  MutexAutoLock lock(mMutex);
+
   MOZ_ASSERT(mLockCount > 0, "VolatileBuffer unlocked too many times!");
   if (--mLockCount || OnHeap()) {
     return;
   }
 
   int state = VM_PURGABLE_VOLATILE | VM_VOLATILE_GROUP_DEFAULT;
   DebugOnly<kern_return_t> ret =
     vm_purgable_control(mach_task_self(),
--- a/memory/volatile/VolatileBufferWindows.cpp
+++ b/memory/volatile/VolatileBufferWindows.cpp
@@ -17,17 +17,18 @@ extern "C" int posix_memalign(void** mem
 #define MEM_RESET_UNDO 0x1000000
 #endif
 
 #define MIN_VOLATILE_ALLOC_SIZE 8192
 
 namespace mozilla {
 
 VolatileBuffer::VolatileBuffer()
-  : mBuf(nullptr)
+  : mMutex("VolatileBuffer")
+  , mBuf(nullptr)
   , mSize(0)
   , mLockCount(0)
   , mHeap(false)
   , mFirstLock(true)
 {
 }
 
 bool
@@ -63,30 +64,34 @@ heap_alloc:
   mBuf = _aligned_malloc(aSize, aAlignment);
 #endif
   mHeap = true;
   return !!mBuf;
 }
 
 VolatileBuffer::~VolatileBuffer()
 {
+  MOZ_ASSERT(mLockCount == 0, "Being destroyed with non-zero lock count?");
+
   if (OnHeap()) {
 #ifdef MOZ_MEMORY
     free(mBuf);
 #else
     _aligned_free(mBuf);
 #endif
   } else {
     VirtualFreeEx(GetCurrentProcess(), mBuf, 0, MEM_RELEASE);
   }
 }
 
 bool
 VolatileBuffer::Lock(void** aBuf)
 {
+  MutexAutoLock lock(mMutex);
+
   MOZ_ASSERT(mBuf, "Attempting to lock an uninitialized VolatileBuffer");
 
   *aBuf = mBuf;
   if (++mLockCount > 1 || OnHeap()) {
     return true;
   }
 
   // MEM_RESET_UNDO's behavior is undefined when called on memory that
@@ -102,16 +107,18 @@ VolatileBuffer::Lock(void** aBuf)
                               MEM_RESET_UNDO,
                               PAGE_READWRITE);
   return !!addr;
 }
 
 void
 VolatileBuffer::Unlock()
 {
+  MutexAutoLock lock(mMutex);
+
   MOZ_ASSERT(mLockCount > 0, "VolatileBuffer unlocked too many times!");
   if (--mLockCount || OnHeap()) {
     return;
   }
 
   void* addr = VirtualAllocEx(GetCurrentProcess(),
                               mBuf,
                               mSize,