From fb673b7c3a42e00dc2d93bdbfa5191bf53d83ce5 Mon Sep 17 00:00:00 2001 From: Seth Fowler Date: Fri, 16 Jan 2015 15:47:35 -0800 Subject: [PATCH] Bug 1121297 (Part 2) - Make VolatileBuffer threadsafe. r=glandium --- memory/volatile/VolatileBuffer.h | 18 ++++++++++++++---- memory/volatile/VolatileBufferAshmem.cpp | 9 ++++++++- memory/volatile/VolatileBufferFallback.cpp | 9 ++++++++- memory/volatile/VolatileBufferOSX.cpp | 9 ++++++++- memory/volatile/VolatileBufferWindows.cpp | 9 ++++++++- 5 files changed, 46 insertions(+), 8 deletions(-) diff --git a/memory/volatile/VolatileBuffer.h b/memory/volatile/VolatileBuffer.h index 76903b2cb16a..e82f755a824b 100644 --- a/memory/volatile/VolatileBuffer.h +++ b/memory/volatile/VolatileBuffer.h @@ -6,6 +6,7 @@ #define mozalloc_VolatileBuffer_h #include "mozilla/mozalloc.h" +#include "mozilla/Mutex.h" #include "mozilla/RefPtr.h" #include "mozilla/MemoryReporting.h" @@ -36,19 +37,19 @@ * 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 +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*)); @@ -62,6 +63,15 @@ protected: 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; diff --git a/memory/volatile/VolatileBufferAshmem.cpp b/memory/volatile/VolatileBufferAshmem.cpp index ea34df263862..09904d6efe01 100644 --- a/memory/volatile/VolatileBufferAshmem.cpp +++ b/memory/volatile/VolatileBufferAshmem.cpp @@ -22,7 +22,8 @@ extern "C" int posix_memalign(void** memptr, size_t alignment, size_t size); namespace mozilla { VolatileBuffer::VolatileBuffer() - : mBuf(nullptr) + : mMutex("VolatileBuffer") + , mBuf(nullptr) , mSize(0) , mLockCount(0) , mFd(-1) @@ -72,6 +73,8 @@ heap_alloc: VolatileBuffer::~VolatileBuffer() { + MOZ_ASSERT(mLockCount == 0, "Being destroyed with non-zero lock count?"); + if (OnHeap()) { free(mBuf); } else { @@ -83,6 +86,8 @@ VolatileBuffer::~VolatileBuffer() bool VolatileBuffer::Lock(void** aBuf) { + MutexAutoLock lock(mMutex); + MOZ_ASSERT(mBuf, "Attempting to lock an uninitialized VolatileBuffer"); *aBuf = mBuf; @@ -98,6 +103,8 @@ VolatileBuffer::Lock(void** aBuf) void VolatileBuffer::Unlock() { + MutexAutoLock lock(mMutex); + MOZ_ASSERT(mLockCount > 0, "VolatileBuffer unlocked too many times!"); if (--mLockCount || OnHeap()) { return; diff --git a/memory/volatile/VolatileBufferFallback.cpp b/memory/volatile/VolatileBufferFallback.cpp index f1d1bfd53676..f4bfe39c6487 100644 --- a/memory/volatile/VolatileBufferFallback.cpp +++ b/memory/volatile/VolatileBufferFallback.cpp @@ -13,7 +13,8 @@ int posix_memalign(void** memptr, size_t alignment, size_t size); namespace mozilla { VolatileBuffer::VolatileBuffer() - : mBuf(nullptr) + : mMutex("VolatileBuffer") + , mBuf(nullptr) , mSize(0) , mLockCount(0) { @@ -42,12 +43,16 @@ bool VolatileBuffer::Init(size_t aSize, size_t aAlignment) 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; @@ -59,6 +64,8 @@ VolatileBuffer::Lock(void** aBuf) void VolatileBuffer::Unlock() { + MutexAutoLock lock(mMutex); + mLockCount--; MOZ_ASSERT(mLockCount >= 0, "VolatileBuffer unlocked too many times!"); } diff --git a/memory/volatile/VolatileBufferOSX.cpp b/memory/volatile/VolatileBufferOSX.cpp index 3f567d4f35ba..af39bcae166b 100644 --- a/memory/volatile/VolatileBufferOSX.cpp +++ b/memory/volatile/VolatileBufferOSX.cpp @@ -16,7 +16,8 @@ namespace mozilla { VolatileBuffer::VolatileBuffer() - : mBuf(nullptr) + : mMutex("VolatileBuffer") + , mBuf(nullptr) , mSize(0) , mLockCount(0) , mHeap(false) @@ -53,6 +54,8 @@ heap_alloc: VolatileBuffer::~VolatileBuffer() { + MOZ_ASSERT(mLockCount == 0, "Being destroyed with non-zero lock count?"); + if (OnHeap()) { free(mBuf); } else { @@ -63,6 +66,8 @@ VolatileBuffer::~VolatileBuffer() bool VolatileBuffer::Lock(void** aBuf) { + MutexAutoLock lock(mMutex); + MOZ_ASSERT(mBuf, "Attempting to lock an uninitialized VolatileBuffer"); *aBuf = mBuf; @@ -82,6 +87,8 @@ VolatileBuffer::Lock(void** aBuf) void VolatileBuffer::Unlock() { + MutexAutoLock lock(mMutex); + MOZ_ASSERT(mLockCount > 0, "VolatileBuffer unlocked too many times!"); if (--mLockCount || OnHeap()) { return; diff --git a/memory/volatile/VolatileBufferWindows.cpp b/memory/volatile/VolatileBufferWindows.cpp index c7324b0cec35..b12e0eccb533 100644 --- a/memory/volatile/VolatileBufferWindows.cpp +++ b/memory/volatile/VolatileBufferWindows.cpp @@ -22,7 +22,8 @@ extern "C" int posix_memalign(void** memptr, size_t alignment, size_t size); namespace mozilla { VolatileBuffer::VolatileBuffer() - : mBuf(nullptr) + : mMutex("VolatileBuffer") + , mBuf(nullptr) , mSize(0) , mLockCount(0) , mHeap(false) @@ -68,6 +69,8 @@ heap_alloc: VolatileBuffer::~VolatileBuffer() { + MOZ_ASSERT(mLockCount == 0, "Being destroyed with non-zero lock count?"); + if (OnHeap()) { #ifdef MOZ_MEMORY free(mBuf); @@ -82,6 +85,8 @@ VolatileBuffer::~VolatileBuffer() bool VolatileBuffer::Lock(void** aBuf) { + MutexAutoLock lock(mMutex); + MOZ_ASSERT(mBuf, "Attempting to lock an uninitialized VolatileBuffer"); *aBuf = mBuf; @@ -107,6 +112,8 @@ VolatileBuffer::Lock(void** aBuf) void VolatileBuffer::Unlock() { + MutexAutoLock lock(mMutex); + MOZ_ASSERT(mLockCount > 0, "VolatileBuffer unlocked too many times!"); if (--mLockCount || OnHeap()) { return;