Bug 637461 - data race in StartupCache.cpp; r=mwu
authorTaras Glek <tglek@mozilla.com>
Fri, 01 Apr 2011 01:24:16 -0400
changeset 64597 e83c07cdbd9b858554a68601f8445d5f8abaa912
parent 64596 8bda9434ed31c847d0b18c138e8b927bb718e58c
child 64598 7b488076916772df45fc24e84f8fffcfba8f978e
push idunknown
push userunknown
push dateunknown
reviewersmwu
bugs637461
milestone2.2a1pre
Bug 637461 - data race in StartupCache.cpp; r=mwu
startupcache/StartupCache.cpp
--- a/startupcache/StartupCache.cpp
+++ b/startupcache/StartupCache.cpp
@@ -60,17 +60,19 @@
 #include "nsITimer.h"
 #include "nsIZipWriter.h"
 #include "nsIZipReader.h"
 #include "nsWeakReference.h"
 #include "nsZipArchive.h"
 #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
 #define SC_ENDIAN "little"
 #endif
 
 #if PR_BYTES_PER_WORD == 4
 #define SC_WORDSIZE "4"
@@ -121,28 +123,31 @@ StartupCache::StartupCache()
 
 StartupCache::~StartupCache() 
 {
   if (mTimer) {
     mTimer->Cancel();
   }
 
   // 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;
 }
 
 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
   mWriteObjectMap.Init();
 #endif
 
   // This allows to override the startup cache filename
   // which is useful from xpcshell, when there is no ProfLDS directory to keep cache in.
@@ -196,35 +201,38 @@ StartupCache::Init()
   // If it's corrupted, just remove it and start over.
   if (NS_FAILED(rv) && rv != NS_ERROR_FILE_NOT_FOUND) {
     NS_WARNING("Failed to load startupcache file correctly, removing!");
     InvalidateCache();
   }
   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);
   if (NS_FAILED(rv) || !exists)
     return NS_ERROR_FILE_NOT_FOUND;
   
   mArchive = new nsZipArchive();
   return mArchive->OpenArchive(mFile);
 }
 
 // NOTE: this will not find a new entry until it has been written to disk!
 // Consumer should take ownership of the resulting buffer.
 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; 
     nsDependentCString idStr(id);
     mTable.Get(idStr, &entry);
     if (entry) {
       *outbuf = new char[entry->size];
       memcpy(*outbuf, entry->data, entry->size);
@@ -255,16 +263,17 @@ StartupCache::GetBuffer(const char* id, 
 #endif
   return NS_ERROR_NOT_AVAILABLE;
 }
 
 // Makes a copy of the buffer, client retains ownership of inbuf.
 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;
   }
 
   nsAutoArrayPtr<char> data(new char[len]);
   memcpy(data, inbuf, len);
 
@@ -315,20 +324,24 @@ CacheCloseHelper(const nsACString& key, 
   rv = writer->AddEntryStream(key, holder->time, PR_TRUE, stream, PR_FALSE);
   
   if (NS_FAILED(rv)) {
     NS_WARNING("cache entry deleted but not written to disk.");
   }
   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;
 
   if (mTable.Count() == 0)
     return;
 
   nsCOMPtr<nsIZipWriter> zipW = do_CreateInstance("@mozilla.org/zipwriter;1");
   if (!zipW)
@@ -377,23 +390,22 @@ StartupCache::InvalidateCache()
 /*
  * WaitOnWriteThread() is called from a main thread to wait for the worker
  * thread to finish. However since the same code is used in the worker thread and
  * main thread, the worker thread can also call WaitOnWriteThread() which is a no-op.
  */
 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;
 }
 
 void 
 StartupCache::ThreadedWrite(void *aClosure)
 {
   gStartupCache->WriteToDisk();
 }