Bug 784657 - Fix potential problem in nsDIskCacheMap timer handling. r=michal
authorBrian R. Bondy <netzen@gmail.com>
Thu, 23 Aug 2012 22:23:10 -0400
changeset 103274 7d83016f15625673de05422434e577560c16d2d1
parent 103273 281b36542acd08890101493a1bac38af711e42db
child 103275 b9d4600374dcf7c99d4b56d7fa3aa33a8eef709b
push id23343
push userryanvm@gmail.com
push dateSat, 25 Aug 2012 02:53:35 +0000
treeherdermozilla-central@f077de66e52d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmichal
bugs784657
milestone17.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 784657 - Fix potential problem in nsDIskCacheMap timer handling. r=michal
netwerk/cache/nsDiskCacheDevice.h
netwerk/cache/nsDiskCacheMap.cpp
toolkit/components/telemetry/TelemetryHistograms.h
--- a/netwerk/cache/nsDiskCacheDevice.h
+++ b/netwerk/cache/nsDiskCacheDevice.h
@@ -68,16 +68,17 @@ public:
     uint32_t                getCacheSize();
     uint32_t                getEntryCount();
     
     nsDiskCacheMap *        CacheMap()    { return &mCacheMap; }
     
 private:    
     friend class nsDiskCacheDeviceDeactivateEntryEvent;
     friend class nsEvictDiskCacheEntriesEvent;
+    friend class nsDiskCacheMap;
     /**
      *  Private methods
      */
 
     inline bool IsValidBinding(nsDiskCacheBinding *binding)
     {
         NS_ASSERTION(binding, "  binding == nullptr");
         NS_ASSERTION(binding->mDeactivateEvent == nullptr,
--- a/netwerk/cache/nsDiskCacheMap.cpp
+++ b/netwerk/cache/nsDiskCacheMap.cpp
@@ -2,16 +2,17 @@
 /* vim:set ts=4 sw=4 sts=4 cin et: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsDiskCacheMap.h"
 #include "nsDiskCacheBinding.h"
 #include "nsDiskCacheEntry.h"
+#include "nsDiskCacheDevice.h"
 #include "nsCacheService.h"
 
 #include "nsCache.h"
 
 #include <string.h>
 #include "nsPrintfCString.h"
 
 #include "nsISerializable.h"
@@ -1212,17 +1213,17 @@ nsDiskCacheMap::NotifyCapacityChange(uin
 nsresult
 nsDiskCacheMap::InitCacheClean(nsIFile *  cacheDirectory,
                                nsDiskCache::CorruptCacheInfo *  corruptInfo)
 {
     // The _CACHE_CLEAN_ file will be used in the future to determine
     // if the cache is clean or not. 
     bool cacheCleanFileExists = false;
     nsCOMPtr<nsIFile> cacheCleanFile;
-    nsresult rv = cacheDirectory->Clone(getter_AddRefs(cacheCleanFile));
+    nsresult rv = cacheDirectory->GetParent(getter_AddRefs(cacheCleanFile));
     if (NS_SUCCEEDED(rv)) {
         rv = cacheCleanFile->AppendNative(
                  NS_LITERAL_CSTRING("_CACHE_CLEAN_"));
         if (NS_SUCCEEDED(rv)) {
             // Check if the file already exists, if it does, we will later read the
             // value and report it to telemetry.
             cacheCleanFile->Exists(&cacheCleanFileExists);
         }
@@ -1322,50 +1323,52 @@ nsDiskCacheMap::InvalidateCache()
 }
 
 nsresult
 nsDiskCacheMap::ResetCacheTimer(int32_t timeout)
 {
     mCleanCacheTimer->Cancel();
     nsresult rv =
       mCleanCacheTimer->InitWithFuncCallback(RevalidateTimerCallback,
-                                             this, timeout,
+                                             nullptr, timeout,
                                              nsITimer::TYPE_ONE_SHOT);
     NS_ENSURE_SUCCESS(rv, rv);
     mLastInvalidateTime = PR_IntervalNow();
 
     return rv;
 }
 
 void
 nsDiskCacheMap::RevalidateTimerCallback(nsITimer *aTimer, void *arg)
 {
-    nsDiskCacheMap *diskCacheMap = reinterpret_cast<nsDiskCacheMap *>(arg);
-    nsresult rv;
-
-    // Intentional braces to scope mutex to only what is needed
-    {
-        nsCacheServiceAutoLock lock(LOCK_TELEM(NSDISKCACHEMAP_REVALIDATION));
-        // If we have less than kLastInvalidateTime since the last timer was
-        // issued then another thread called InvalidateCache.  This won't catch
-        // all cases where we wanted to cancel the timer, but under the lock it
-        // is always OK to revalidate as long as IsCacheInSafeState() returns
-        // true.  We just want to avoid revalidating when we can to reduce IO
-        // and this check will do that.
-        uint32_t delta =
-            PR_IntervalToMilliseconds(PR_IntervalNow() -
-                                      diskCacheMap->mLastInvalidateTime) +
-            kRevalidateCacheTimeoutTolerance;
-        if (delta < kRevalidateCacheTimeout) {
-            diskCacheMap->ResetCacheTimer();
-            return;
-        }
-        rv = diskCacheMap->RevalidateCache();
+    nsCacheServiceAutoLock lock(LOCK_TELEM(NSDISKCACHEMAP_REVALIDATION));
+    if (!nsCacheService::gService->mDiskDevice ||
+        !nsCacheService::gService->mDiskDevice->Initialized()) {
+        return;
     }
 
+    nsDiskCacheMap *diskCacheMap =
+        &nsCacheService::gService->mDiskDevice->mCacheMap;
+
+    // If we have less than kRevalidateCacheTimeout since the last timer was
+    // issued then another thread called InvalidateCache.  This won't catch
+    // all cases where we wanted to cancel the timer, but under the lock it
+    // is always OK to revalidate as long as IsCacheInSafeState() returns
+    // true.  We just want to avoid revalidating when we can to reduce IO
+    // and this check will do that.
+    uint32_t delta =
+        PR_IntervalToMilliseconds(PR_IntervalNow() -
+                                  diskCacheMap->mLastInvalidateTime) +
+        kRevalidateCacheTimeoutTolerance;
+    if (delta < kRevalidateCacheTimeout) {
+        diskCacheMap->ResetCacheTimer();
+        return;
+    }
+
+    nsresult rv = diskCacheMap->RevalidateCache();
     if (NS_FAILED(rv)) {
         diskCacheMap->ResetCacheTimer(kRevalidateCacheErrorTimeout);
     }
 }
 
 bool
 nsDiskCacheMap::IsCacheInSafeState()
 {
--- a/toolkit/components/telemetry/TelemetryHistograms.h
+++ b/toolkit/components/telemetry/TelemetryHistograms.h
@@ -196,17 +196,17 @@ HISTOGRAM(SPDY_SETTINGS_MAX_STREAMS, 1, 
 HISTOGRAM(SPDY_SETTINGS_CWND, 1, 500, 50, EXPONENTIAL,  "SPDY: Settings CWND (packets)")
 HISTOGRAM(SPDY_SETTINGS_RETRANS, 1, 100, 50, EXPONENTIAL,  "SPDY: Retransmission Rate")
 HISTOGRAM(SPDY_SETTINGS_IW, 1, 1000, 50, EXPONENTIAL,  "SPDY: Settings IW (rounded to KB)")
 
 #undef _HTTP_HIST
 #undef HTTP_HISTOGRAMS
 
 HISTOGRAM_ENUMERATED_VALUES(DISK_CACHE_CORRUPT_DETAILS, 50, "Why the HTTP disk cache was corrupted at startup")
-HISTOGRAM_BOOLEAN(DISK_CACHE_REDUCTION_TRIAL, "Stores 1 if the cache would be corrupted with the disk cache corruption plan of Bug 105843")
+HISTOGRAM_BOOLEAN(DISK_CACHE_REDUCTION_TRIAL, "Stores 1 if the cache would be clean with the disk cache corruption plan of Bug 105843")
 HISTOGRAM_ENUMERATED_VALUES(HTTP_CACHE_DISPOSITION_2,         5, "HTTP Cache Hit, Reval, Failed-Reval, Miss")
 HISTOGRAM_ENUMERATED_VALUES(HTTP_DISK_CACHE_DISPOSITION_2,    5, "HTTP Disk Cache Hit, Reval, Failed-Reval, Miss")
 HISTOGRAM_ENUMERATED_VALUES(HTTP_MEMORY_CACHE_DISPOSITION_2,  5, "HTTP Memory Cache Hit, Reval, Failed-Reval, Miss")
 HISTOGRAM_ENUMERATED_VALUES(HTTP_OFFLINE_CACHE_DISPOSITION_2, 5, "HTTP Offline Cache Hit, Reval, Failed-Reval, Miss")
 HISTOGRAM(CACHE_DEVICE_SEARCH_2, 1, 10000, 50, EXPONENTIAL, "Time to search cache (ms)")
 HISTOGRAM(CACHE_MEMORY_SEARCH_2, 1, 10000, 50, EXPONENTIAL, "Time to search memory cache (ms)")
 HISTOGRAM(CACHE_DISK_SEARCH_2, 1, 10000, 50, EXPONENTIAL, "Time to search disk cache (ms)")
 HISTOGRAM(CACHE_OFFLINE_SEARCH_2, 1, 10000, 50, EXPONENTIAL, "Time to search offline cache (ms)")