Bug 992512 - DOMStorageCache crashes (sDatabase access violation read), patch v3, r=smaug
authorHonza Bambas <honzab.moz@firemni.cz>
Sat, 26 Apr 2014 21:55:20 +0200
changeset 198928 565704a273f5a59dec8039824d51104f467bcbd5
parent 198927 85e868943b6110a790aaf9141f066ff880251a81
child 198929 fa70674b866e20348af1caf0c17c0a5cd647c454
push id3624
push userasasaki@mozilla.com
push dateMon, 09 Jun 2014 21:49:01 +0000
treeherdermozilla-beta@b1a5da15899a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs992512
milestone31.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 992512 - DOMStorageCache crashes (sDatabase access violation read), patch v3, r=smaug
dom/src/storage/DOMStorageCache.cpp
dom/src/storage/DOMStorageCache.h
--- a/dom/src/storage/DOMStorageCache.cpp
+++ b/dom/src/storage/DOMStorageCache.cpp
@@ -18,16 +18,17 @@
 
 namespace mozilla {
 namespace dom {
 
 #define DOM_STORAGE_CACHE_KEEP_ALIVE_TIME_MS 20000
 
 // static
 DOMStorageDBBridge* DOMStorageCache::sDatabase = nullptr;
+bool DOMStorageCache::sDatabaseDown = false;
 
 namespace { // anon
 
 const uint32_t kDefaultSet = 0;
 const uint32_t kPrivateSet = 1;
 const uint32_t kSessionSet = 2;
 
 inline uint32_t
@@ -335,16 +336,20 @@ DOMStorageCache::WaitForPreload(Telemetr
   // Measure which operation blocks and for how long
   TelemetryAutoTimer timer(aTelemetryID);
 
   // If preload already started (i.e. we got some first data, but not all)
   // SyncPreload will just wait for it to finish rather then synchronously
   // read from the database.  It seems to me more optimal.
 
   // TODO place for A/B testing (force main thread load vs. let preload finish)
+
+  // No need to check sDatabase for being non-null since preload is either
+  // done before we've shut the DB down or when the DB could not start,
+  // preload has not even be started.
   sDatabase->SyncPreload(this);
 }
 
 nsresult
 DOMStorageCache::GetLength(const DOMStorage* aStorage, uint32_t* aRetval)
 {
   Telemetry::AutoTimer<Telemetry::LOCALDOMSTORAGE_GETLENGTH_MS> autoTimer;
 
@@ -491,16 +496,22 @@ DOMStorageCache::SetItem(const DOMStorag
 
   if (aValue == aOld && DOMStringIsNull(aValue) == DOMStringIsNull(aOld)) {
     return NS_SUCCESS_DOM_NO_OPERATION;
   }
 
   data.mKeys.Put(aKey, aValue);
 
   if (Persist(aStorage)) {
+    if (!sDatabase) {
+      NS_ERROR("Writing to localStorage after the database has been shut down"
+               ", data lose!");
+      return NS_ERROR_NOT_INITIALIZED;
+    }
+
     if (DOMStringIsNull(aOld)) {
       return sDatabase->AsyncAddItem(this, aKey, aValue);
     }
 
     return sDatabase->AsyncUpdateItem(this, aKey, aValue);
   }
 
   return NS_OK;
@@ -526,16 +537,22 @@ DOMStorageCache::RemoveItem(const DOMSto
   }
 
   // Recalculate the cached data size
   const int64_t delta = -(static_cast<int64_t>(aOld.Length()));
   unused << ProcessUsageDelta(aStorage, delta);
   data.mKeys.Remove(aKey);
 
   if (Persist(aStorage)) {
+    if (!sDatabase) {
+      NS_ERROR("Writing to localStorage after the database has been shut down"
+               ", data lose!");
+      return NS_ERROR_NOT_INITIALIZED;
+    }
+
     return sDatabase->AsyncRemoveItem(this, aKey);
   }
 
   return NS_OK;
 }
 
 nsresult
 DOMStorageCache::Clear(const DOMStorage* aStorage)
@@ -562,16 +579,22 @@ DOMStorageCache::Clear(const DOMStorage*
   bool hadData = !!data.mKeys.Count();
 
   if (hadData) {
     unused << ProcessUsageDelta(aStorage, -data.mOriginQuotaUsage);
     data.mKeys.Clear();
   }
 
   if (Persist(aStorage) && (refresh || hadData)) {
+    if (!sDatabase) {
+      NS_ERROR("Writing to localStorage after the database has been shut down"
+               ", data lose!");
+      return NS_ERROR_NOT_INITIALIZED;
+    }
+
     return sDatabase->AsyncClear(this);
   }
 
   return hadData ? NS_OK : NS_SUCCESS_DOM_NO_OPERATION;
 }
 
 void
 DOMStorageCache::CloneFrom(const DOMStorageCache* aThat)
@@ -740,17 +763,20 @@ DOMStorageUsage::CheckAndSetETLD1UsageDe
   return true;
 }
 
 
 // static
 DOMStorageDBBridge*
 DOMStorageCache::StartDatabase()
 {
-  if (sDatabase) {
+  if (sDatabase || sDatabaseDown) {
+    // When sDatabaseDown is at true, sDatabase is null.
+    // Checking sDatabaseDown flag here prevents reinitialization of
+    // the database after shutdown.
     return sDatabase;
   }
 
   if (XRE_GetProcessType() == GeckoProcessType_Default) {
     nsAutoPtr<DOMStorageDBThread> db(new DOMStorageDBThread());
 
     nsresult rv = db->Init();
     if (NS_FAILED(rv)) {
@@ -783,16 +809,18 @@ DOMStorageCache::GetDatabase()
 // static
 nsresult
 DOMStorageCache::StopDatabase()
 {
   if (!sDatabase) {
     return NS_OK;
   }
 
+  sDatabaseDown = true;
+
   nsresult rv = sDatabase->Shutdown();
   if (XRE_GetProcessType() == GeckoProcessType_Default) {
     delete sDatabase;
   } else {
     DOMStorageDBChild* child = static_cast<DOMStorageDBChild*>(sDatabase);
     NS_RELEASE(child);
   }
 
--- a/dom/src/storage/DOMStorageCache.h
+++ b/dom/src/storage/DOMStorageCache.h
@@ -220,16 +220,19 @@ private:
   bool mSessionOnlyDataSetActive : 1;
 
   // Whether we have already captured state of the cache preload on our first access.
   bool mPreloadTelemetryRecorded : 1;
 
   // DOMStorageDBThread on the parent or single process,
   // DOMStorageDBChild on the child process.
   static DOMStorageDBBridge* sDatabase;
+
+  // False until we shut the database down.
+  static bool sDatabaseDown;
 };
 
 // DOMStorageUsage
 // Infrastructure to manage and check eTLD+1 quota
 class DOMStorageUsageBridge
 {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DOMStorageUsageBridge)