Bug 637461 - data race in StartupCache.cpp; r=mwu

startupcache/StartupCache.cpp

@@ -65,7 +65,9 @@
 #include "mozilla/Omnijar.h"
 #include "prenv.h"
 #include "mozilla/FunctionTimer.h"
- 
+#include "nsThreadUtils.h"
+#include "nsXULAppAPI.h"
+
 #ifdef IS_BIG_ENDIAN
 #define SC_ENDIAN "big"
 #else
@@ -126,11 +128,9 @@ StartupCache::~StartupCache()
   }
 
   // Generally, the in-memory table should be empty here,
-  // but in special cases (like Talos Ts tests) we
-  // could shut down before we write.
-  // This mechanism will change when IO is moved off-thread
-  // (bug 586859) or when Talos first-run is changed to allow
-  // our timer to work (bug 591471).
+  // but an early shutdown means either mTimer didn't run 
+  // or the write thread is still running.
+  WaitOnWriteThread();
   WriteToDisk();
   gStartupCache = nsnull;
 }
@@ -138,6 +138,11 @@ StartupCache::~StartupCache()
 nsresult
 StartupCache::Init() 
 {
+  if (XRE_GetProcessType() != GeckoProcessType_Default) {
+    NS_WARNING("Startup cache is only available in the chrome process");
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
   nsresult rv;
   mTable.Init();
 #ifdef DEBUG
@@ -201,10 +206,12 @@ StartupCache::Init()
   return NS_OK;
 }
 
+/** 
+ * LoadArchive can be called from the main thread or while reloading cache on write thread.
+ */
 nsresult
 StartupCache::LoadArchive() 
 {
-  WaitOnWriteThread();
   PRBool exists;
   mArchive = NULL;
   nsresult rv = mFile->Exists(&exists);
@@ -220,6 +227,7 @@ StartupCache::LoadArchive()
 nsresult
 StartupCache::GetBuffer(const char* id, char** outbuf, PRUint32* length) 
 {
+  NS_ASSERTION(NS_IsMainThread(), "Startup cache only available on main thread");
   WaitOnWriteThread();
   if (!mStartupWriteInitiated) {
     CacheEntry* entry; 
@@ -260,6 +268,7 @@ StartupCache::GetBuffer(const char* id, char** outbuf, PRUint32* length)
 nsresult
 StartupCache::PutBuffer(const char* id, const char* inbuf, PRUint32 len) 
 {
+  NS_ASSERTION(NS_IsMainThread(), "Startup cache only available on main thread");
   WaitOnWriteThread();
   if (StartupCache::gShutdownInitiated) {
     return NS_ERROR_NOT_AVAILABLE;
@@ -320,10 +329,14 @@ CacheCloseHelper(const nsACString& key, nsAutoPtr<CacheEntry>& data,
   return PL_DHASH_REMOVE;
 }
 
+
+/** 
+ * WriteToDisk writes the cache out to disk. Callers of WriteToDisk need to call WaitOnWriteThread
+ * to make sure there isn't a write happening on another thread
+ */
 void
 StartupCache::WriteToDisk() 
 {
-  WaitOnWriteThread();
   nsresult rv;
   mStartupWriteInitiated = PR_TRUE;
 
@@ -382,13 +395,12 @@ StartupCache::InvalidateCache()
 void
 StartupCache::WaitOnWriteThread()
 {
-  PRThread* writeThread = mWriteThread;
-  if (!writeThread || writeThread == PR_GetCurrentThread())
+  NS_ASSERTION(NS_IsMainThread(), "Startup cache should only wait for io thread on main thread");
+  if (!mWriteThread || mWriteThread == PR_GetCurrentThread())
     return;
 
   NS_TIME_FUNCTION_MIN(30);
-  //NS_WARNING("Waiting on startupcache write");
-  PR_JoinThread(writeThread);
+  PR_JoinThread(mWriteThread);
   mWriteThread = NULL;
 }