Bug 505857 - Fix up locking in the category manager, r=timeless
--- a/xpcom/components/nsCategoryManager.cpp
+++ b/xpcom/components/nsCategoryManager.cpp
@@ -56,16 +56,17 @@
#include "nsIObserverService.h"
#include "nsReadableUtils.h"
#include "nsCRT.h"
#include "nsQuickSort.h"
#include "nsEnumeratorUtils.h"
#include "nsIProxyObjectManager.h"
#include "nsThreadUtils.h"
+using namespace mozilla;
class nsIComponentLoaderManager;
/*
CategoryDatabase
contains 0 or more 1-1 mappings of string to Category
each Category contains 0 or more 1-1 mappings of string keys to string values
In other words, the CategoryDatabase is a tree, whose root is a hashtable.
@@ -237,68 +238,59 @@ CategoryNode::Create(PLArenaPool* aArena
if (!node)
return nsnull;
if (!node->mTable.Init()) {
delete node;
return nsnull;
}
- node->mLock = PR_NewLock();
- if (!node->mLock) {
- delete node;
- return nsnull;
- }
-
return node;
}
CategoryNode::~CategoryNode()
{
- if (mLock)
- PR_DestroyLock(mLock);
}
void*
CategoryNode::operator new(size_t aSize, PLArenaPool* aArena)
{
void* p;
PL_ARENA_ALLOCATE(p, aArena, aSize);
return p;
}
NS_METHOD
CategoryNode::GetLeaf(const char* aEntryName,
char** _retval)
{
- PR_Lock(mLock);
+ MutexAutoLock lock(mLock);
nsresult rv = NS_ERROR_NOT_AVAILABLE;
CategoryLeaf* ent =
mTable.GetEntry(aEntryName);
// we only want the non-persistent value
if (ent && ent->nonpValue) {
*_retval = NS_strdup(ent->nonpValue);
if (*_retval)
rv = NS_OK;
}
- PR_Unlock(mLock);
return rv;
}
NS_METHOD
CategoryNode::AddLeaf(const char* aEntryName,
const char* aValue,
PRBool aPersist,
PRBool aReplace,
char** _retval,
PLArenaPool* aArena)
{
- PR_Lock(mLock);
+ MutexAutoLock lock(mLock);
CategoryLeaf* leaf =
mTable.GetEntry(aEntryName);
nsresult rv = NS_OK;
if (leaf) {
//if the entry was found, aReplace must be specified
if (!aReplace && (leaf->nonpValue || (aPersist && leaf->pValue )))
rv = NS_ERROR_INVALID_ARG;
@@ -331,27 +323,26 @@ CategoryNode::AddLeaf(const char* aEntry
}
leaf->nonpValue = arenaValue;
if (aPersist)
leaf->pValue = arenaValue;
}
}
- PR_Unlock(mLock);
return rv;
}
NS_METHOD
CategoryNode::DeleteLeaf(const char* aEntryName,
PRBool aDontPersist)
{
// we don't throw any errors, because it normally doesn't matter
// and it makes JS a lot cleaner
- PR_Lock(mLock);
+ MutexAutoLock lock(mLock);
if (aDontPersist) {
// we can just remove the entire hash entry without introspection
mTable.RemoveEntry(aEntryName);
} else {
// if we are keeping the persistent value, we need to look at
// the contents of the current entry
CategoryLeaf* leaf = mTable.GetEntry(aEntryName);
@@ -359,29 +350,27 @@ CategoryNode::DeleteLeaf(const char* aEn
if (leaf->pValue) {
leaf->nonpValue = nsnull;
} else {
// if there is no persistent value, just remove the entry
mTable.RawRemoveEntry(leaf);
}
}
}
- PR_Unlock(mLock);
return NS_OK;
}
NS_METHOD
CategoryNode::Enumerate(nsISimpleEnumerator **_retval)
{
NS_ENSURE_ARG_POINTER(_retval);
- PR_Lock(mLock);
+ MutexAutoLock lock(mLock);
EntryEnumerator* enumObj = EntryEnumerator::Create(mTable);
- PR_Unlock(mLock);
if (!enumObj)
return NS_ERROR_OUT_OF_MEMORY;
*_retval = enumObj;
NS_ADDREF(*_retval);
return NS_OK;
}
@@ -418,19 +407,18 @@ PRBool
CategoryNode::WritePersistentEntries(PRFileDesc* fd, const char* aCategoryName)
{
persistent_userstruct args = {
fd,
aCategoryName,
PR_TRUE
};
- PR_Lock(mLock);
+ MutexAutoLock lock(mLock);
mTable.EnumerateEntries(enumfunc_pentries, &args);
- PR_Unlock(mLock);
return args.success;
}
//
// CategoryEnumerator class
//
@@ -496,31 +484,21 @@ nsCategoryManager::Create()
PL_INIT_ARENA_POOL(&(manager->mArena), "CategoryManagerArena",
NS_CATEGORYMANAGER_ARENA_SIZE); // this never fails
if (!manager->mTable.Init()) {
delete manager;
return nsnull;
}
- manager->mLock = PR_NewLock();
-
- if (!manager->mLock) {
- delete manager;
- return nsnull;
- }
-
return manager;
}
nsCategoryManager::~nsCategoryManager()
{
- if (mLock)
- PR_DestroyLock(mLock);
-
// the hashtable contains entries that must be deleted before the arena is
// destroyed, or else you will have PRLocks undestroyed and other Really
// Bad Stuff (TM)
mTable.Clear();
PL_FinishArenaPool(&mArena);
}
@@ -579,19 +557,21 @@ nsCategoryManager::GetCategoryEntry( con
char **_retval )
{
NS_ENSURE_ARG_POINTER(aCategoryName);
NS_ENSURE_ARG_POINTER(aEntryName);
NS_ENSURE_ARG_POINTER(_retval);
nsresult status = NS_ERROR_NOT_AVAILABLE;
- PR_Lock(mLock);
- CategoryNode* category = get_category(aCategoryName);
- PR_Unlock(mLock);
+ CategoryNode* category;
+ {
+ MutexAutoLock lock(mLock);
+ category = get_category(aCategoryName);
+ }
if (category) {
status = category->GetLeaf(aEntryName, _retval);
}
return status;
}
@@ -604,27 +584,29 @@ nsCategoryManager::AddCategoryEntry( con
char **_retval )
{
NS_ENSURE_ARG_POINTER(aCategoryName);
NS_ENSURE_ARG_POINTER(aEntryName);
NS_ENSURE_ARG_POINTER(aValue);
// Before we can insert a new entry, we'll need to
// find the |CategoryNode| to put it in...
- PR_Lock(mLock);
- CategoryNode* category = get_category(aCategoryName);
+ CategoryNode* category;
+ {
+ MutexAutoLock lock(mLock);
+ category = get_category(aCategoryName);
- if (!category) {
- // That category doesn't exist yet; let's make it.
- category = CategoryNode::Create(&mArena);
+ if (!category) {
+ // That category doesn't exist yet; let's make it.
+ category = CategoryNode::Create(&mArena);
- char* categoryName = ArenaStrdup(aCategoryName, &mArena);
- mTable.Put(categoryName, category);
+ char* categoryName = ArenaStrdup(aCategoryName, &mArena);
+ mTable.Put(categoryName, category);
+ }
}
- PR_Unlock(mLock);
if (!category)
return NS_ERROR_OUT_OF_MEMORY;
// We will need the return value of AddLeaf even if the called doesn't want it
char *oldEntry = nsnull;
nsresult rv = category->AddLeaf(aEntryName,
@@ -660,19 +642,21 @@ nsCategoryManager::DeleteCategoryEntry(
NS_ENSURE_ARG_POINTER(aEntryName);
/*
Note: no errors are reported since failure to delete
probably won't hurt you, and returning errors seriously
inconveniences JS clients
*/
- PR_Lock(mLock);
- CategoryNode* category = get_category(aCategoryName);
- PR_Unlock(mLock);
+ CategoryNode* category;
+ {
+ MutexAutoLock lock(mLock);
+ category = get_category(aCategoryName);
+ }
if (!category)
return NS_OK;
nsresult rv = category->DeleteLeaf(aEntryName,
aDontPersist);
if (NS_SUCCEEDED(rv)) {
@@ -687,19 +671,21 @@ NS_IMETHODIMP
nsCategoryManager::DeleteCategory( const char *aCategoryName )
{
NS_ENSURE_ARG_POINTER(aCategoryName);
// the categories are arena-allocated, so we don't
// actually delete them. We just remove all of the
// leaf nodes.
- PR_Lock(mLock);
- CategoryNode* category = get_category(aCategoryName);
- PR_Unlock(mLock);
+ CategoryNode* category;
+ {
+ MutexAutoLock lock(mLock);
+ category = get_category(aCategoryName);
+ }
if (category) {
category->Clear();
NotifyObservers(NS_XPCOM_CATEGORY_CLEARED_OBSERVER_ID,
aCategoryName, nsnull);
}
return NS_OK;
@@ -707,35 +693,36 @@ nsCategoryManager::DeleteCategory( const
NS_IMETHODIMP
nsCategoryManager::EnumerateCategory( const char *aCategoryName,
nsISimpleEnumerator **_retval )
{
NS_ENSURE_ARG_POINTER(aCategoryName);
NS_ENSURE_ARG_POINTER(_retval);
- PR_Lock(mLock);
- CategoryNode* category = get_category(aCategoryName);
- PR_Unlock(mLock);
+ CategoryNode* category;
+ {
+ MutexAutoLock lock(mLock);
+ category = get_category(aCategoryName);
+ }
if (!category) {
return NS_NewEmptyEnumerator(_retval);
}
return category->Enumerate(_retval);
}
NS_IMETHODIMP
nsCategoryManager::EnumerateCategories(nsISimpleEnumerator **_retval)
{
NS_ENSURE_ARG_POINTER(_retval);
- PR_Lock(mLock);
+ MutexAutoLock lock(mLock);
CategoryEnumerator* enumObj = CategoryEnumerator::Create(mTable);
- PR_Unlock(mLock);
if (!enumObj)
return NS_ERROR_OUT_OF_MEMORY;
*_retval = enumObj;
NS_ADDREF(*_retval);
return NS_OK;
}
@@ -763,19 +750,18 @@ enumfunc_categories(const char* aKey, Ca
NS_METHOD
nsCategoryManager::WriteCategoryManagerToRegistry(PRFileDesc* fd)
{
writecat_struct args = {
fd,
PR_TRUE
};
- PR_Lock(mLock);
+ MutexAutoLock lock(mLock);
mTable.EnumerateRead(enumfunc_categories, &args);
- PR_Unlock(mLock);
if (!args.success) {
return NS_ERROR_UNEXPECTED;
}
return NS_OK;
}
--- a/xpcom/components/nsCategoryManager.h
+++ b/xpcom/components/nsCategoryManager.h
@@ -35,20 +35,20 @@
*
* ***** END LICENSE BLOCK ***** */
#ifndef NSCATEGORYMANAGER_H
#define NSCATEGORYMANAGER_H
#include "prio.h"
-#include "prlock.h"
#include "plarena.h"
#include "nsClassHashtable.h"
#include "nsICategoryManager.h"
+#include "mozilla/Mutex.h"
#define NS_CATEGORYMANAGER_CLASSNAME "Category Manager"
/* 16d222a6-1dd2-11b2-b693-f38b02c021b2 */
#define NS_CATEGORYMANAGER_CID \
{ 0x16d222a6, 0x1dd2, 0x11b2, \
{0xb6, 0x93, 0xf3, 0x8b, 0x02, 0xc0, 0x21, 0xb2} }
@@ -89,43 +89,44 @@ public:
PRBool aReplace,
char** _retval,
PLArenaPool* aArena);
NS_METHOD DeleteLeaf(const char* aEntryName,
PRBool aDontPersist);
void Clear() {
- PR_Lock(mLock);
+ mozilla::MutexAutoLock lock(mLock);
mTable.Clear();
- PR_Unlock(mLock);
}
PRUint32 Count() {
- PR_Lock(mLock);
+ mozilla::MutexAutoLock lock(mLock);
PRUint32 tCount = mTable.Count();
- PR_Unlock(mLock);
return tCount;
}
NS_METHOD Enumerate(nsISimpleEnumerator** _retval);
PRBool WritePersistentEntries(PRFileDesc* fd, const char* aCategoryName);
// CategoryNode is arena-allocated, with the strings
static CategoryNode* Create(PLArenaPool* aArena);
~CategoryNode();
void operator delete(void*) { }
private:
- CategoryNode() { }
+ CategoryNode()
+ : mLock("CategoryLeaf")
+ { }
+
void* operator new(size_t aSize, PLArenaPool* aArena);
nsTHashtable<CategoryLeaf> mTable;
- PRLock* mLock;
+ mozilla::Mutex mLock;
};
/**
* The main implementation of nsICategoryManager.
*
* This implementation is thread-safe.
*/
@@ -145,27 +146,30 @@ public:
/**
* Suppress or unsuppress notifications of category changes to the
* observer service. This is to be used by nsComponentManagerImpl
* on startup while reading the stored category list.
*/
NS_METHOD SuppressNotifications(PRBool aSuppress);
nsCategoryManager()
- : mSuppressNotifications(PR_FALSE) { }
+ : mLock("nsCategoryManager")
+ , mSuppressNotifications(PR_FALSE)
+ { }
+
private:
friend class nsCategoryManagerFactory;
static nsCategoryManager* Create();
~nsCategoryManager();
CategoryNode* get_category(const char* aName);
void NotifyObservers(const char* aTopic,
const char* aCategoryName,
const char* aEntryName);
PLArenaPool mArena;
nsClassHashtable<nsDepCharHashKey, CategoryNode> mTable;
- PRLock* mLock;
+ mozilla::Mutex mLock;
PRBool mSuppressNotifications;
};
#endif