Bug 505857 - Fix up locking in the category manager, r=timeless
authorBenjamin Smedberg <benjamin@smedbergs.us>
Thu, 23 Jul 2009 10:29:00 -0400
changeset 30601 d18fc420273906730deb91b81bf7c0dd15547280
parent 30600 eb7644ffddff11d6da837f80c189afad9642b4a3
child 30602 c8a66d96c1a1b94ba893e3c116507622e8c9793f
push idunknown
push userunknown
push dateunknown
reviewerstimeless
bugs505857
milestone1.9.2a1pre
Bug 505857 - Fix up locking in the category manager, r=timeless
xpcom/components/nsCategoryManager.cpp
xpcom/components/nsCategoryManager.h
--- 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