Bug 745141 - crash in imgRequestProxy::OnDiscard with abort message. r=jlebar
authorBrian R. Bondy <netzen@gmail.com>
Tue, 08 May 2012 08:25:08 -0400
changeset 97995 c2ddddc684328e0f1c24f64d5c0b2419ef20cf38
parent 97994 4ab51d9e902bc44162253256b683b4cf3f5a651c
child 97996 4eda2c7b640cc191f95148f79244af8a3936accb
push id173
push userlsblakk@mozilla.com
push dateFri, 24 Aug 2012 15:39:16 +0000
treeherdermozilla-release@bcc45eb1fb41 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlebar
bugs745141
milestone15.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 745141 - crash in imgRequestProxy::OnDiscard with abort message. r=jlebar
image/build/nsImageModule.cpp
image/src/DiscardTracker.cpp
image/src/DiscardTracker.h
image/src/RasterImage.cpp
--- a/image/build/nsImageModule.cpp
+++ b/image/build/nsImageModule.cpp
@@ -123,16 +123,17 @@ static const mozilla::Module::CategoryEn
   { "Gecko-Content-Viewers", "image/x-png", "@mozilla.org/content/document-loader-factory;1" },
   { "content-sniffing-services", "@mozilla.org/image/loader;1", "@mozilla.org/image/loader;1" },
   { NULL }
 };
 
 static nsresult
 imglib_Initialize()
 {
+  mozilla::image::DiscardTracker::Initialize();
   imgLoader::InitCache();
   return NS_OK;
 }
 
 static void
 imglib_Shutdown()
 {
   imgLoader::Shutdown();
--- a/image/src/DiscardTracker.cpp
+++ b/image/src/DiscardTracker.cpp
@@ -13,29 +13,31 @@ namespace mozilla {
 namespace image {
 
 static const char* sDiscardTimeoutPref = "image.mem.min_discard_timeout_ms";
 
 /* static */ LinkedList<DiscardTracker::Node> DiscardTracker::sDiscardableImages;
 /* static */ nsCOMPtr<nsITimer> DiscardTracker::sTimer;
 /* static */ bool DiscardTracker::sInitialized = false;
 /* static */ bool DiscardTracker::sTimerOn = false;
-/* static */ bool DiscardTracker::sDiscardRunnablePending = false;
+/* static */ PRInt32 DiscardTracker::sDiscardRunnablePending = 0;
 /* static */ PRInt64 DiscardTracker::sCurrentDecodedImageBytes = 0;
 /* static */ PRUint32 DiscardTracker::sMinDiscardTimeoutMs = 10000;
 /* static */ PRUint32 DiscardTracker::sMaxDecodedImageKB = 42 * 1024;
+/* static */ PRLock * DiscardTracker::sAllocationLock = NULL;
 
 /*
  * When we notice we're using too much memory for decoded images, we enqueue a
  * DiscardRunnable, which runs this code.
  */
 NS_IMETHODIMP
 DiscardTracker::DiscardRunnable::Run()
 {
-  sDiscardRunnablePending = false;
+  PR_ATOMIC_SET(&sDiscardRunnablePending, 0);
+
   DiscardTracker::DiscardNow();
   return NS_OK;
 }
 
 int
 DiscardTimeoutChangedCallback(const char* aPref, void *aClosure)
 {
   DiscardTracker::ReloadTimeout();
@@ -43,77 +45,78 @@ DiscardTimeoutChangedCallback(const char
 }
 
 nsresult
 DiscardTracker::Reset(Node *node)
 {
   // We shouldn't call Reset() with a null |img| pointer, on images which can't
   // be discarded, or on animated images (which should be marked as
   // non-discardable, anyway).
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(sInitialized);
   MOZ_ASSERT(node->img);
   MOZ_ASSERT(node->img->CanDiscard());
   MOZ_ASSERT(!node->img->mAnim);
 
-  // Initialize the first time through.
-  nsresult rv;
-  if (NS_UNLIKELY(!sInitialized)) {
-    rv = Initialize();
-    NS_ENSURE_SUCCESS(rv, rv);
-  }
-
   // Insert the node at the front of the list and note when it was inserted.
   bool wasInList = node->isInList();
   if (wasInList) {
     node->remove();
   }
   node->timestamp = TimeStamp::Now();
   sDiscardableImages.insertFront(node);
 
   // If the node wasn't already in the list of discardable images, then we may
   // need to discard some images to stay under the sMaxDecodedImageKB limit.
   // Call MaybeDiscardSoon to do this check.
   if (!wasInList) {
     MaybeDiscardSoon();
   }
 
   // Make sure the timer is running.
-  rv = EnableTimer();
+  nsresult rv = EnableTimer();
   NS_ENSURE_SUCCESS(rv,rv);
 
   return NS_OK;
 }
 
 void
 DiscardTracker::Remove(Node *node)
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (node->isInList())
     node->remove();
 
   if (sDiscardableImages.isEmpty())
     DisableTimer();
 }
 
 /**
  * Shut down the tracker, deallocating the timer.
  */
 void
 DiscardTracker::Shutdown()
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (sTimer) {
     sTimer->Cancel();
     sTimer = NULL;
   }
 }
 
 /*
  * Discard all the images we're tracking.
  */
 void
 DiscardTracker::DiscardAll()
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   if (!sInitialized)
     return;
 
   // Be careful: Calling Discard() on an image might cause it to be removed
   // from the list!
   Node *n;
   while ((n = sDiscardableImages.popFirst())) {
     n->img->Discard();
@@ -123,41 +126,50 @@ DiscardTracker::DiscardAll()
   DisableTimer();
 }
 
 void
 DiscardTracker::InformAllocation(PRInt64 bytes)
 {
   // This function is called back e.g. from RasterImage::Discard(); be careful!
 
+  MOZ_ASSERT(sInitialized);
+
+  PR_Lock(sAllocationLock);
   sCurrentDecodedImageBytes += bytes;
   MOZ_ASSERT(sCurrentDecodedImageBytes >= 0);
+  PR_Unlock(sAllocationLock);
 
   // If we're using too much memory for decoded images, MaybeDiscardSoon will
   // enqueue a callback to discard some images.
   MaybeDiscardSoon();
 }
 
 /**
  * Initialize the tracker.
  */
 nsresult
 DiscardTracker::Initialize()
 {
+  MOZ_ASSERT(NS_IsMainThread());
+
   // Watch the timeout pref for changes.
   Preferences::RegisterCallback(DiscardTimeoutChangedCallback,
                                 sDiscardTimeoutPref);
 
   Preferences::AddUintVarCache(&sMaxDecodedImageKB,
                               "image.mem.max_decoded_image_kb",
                               50 * 1024);
 
   // Create the timer.
   sTimer = do_CreateInstance("@mozilla.org/timer;1");
 
+  // Create a lock for safegarding the 64-bit sCurrentDecodedImageBytes
+  sAllocationLock = PR_NewLock();
+
   // Mark us as initialized
   sInitialized = true;
 
   // Read the timeout pref and start the timer.
   ReloadTimeout();
 
   return NS_OK;
 }
@@ -270,17 +282,19 @@ DiscardTracker::DiscardNow()
 }
 
 void
 DiscardTracker::MaybeDiscardSoon()
 {
   // Are we carrying around too much decoded image data?  If so, enqueue an
   // event to try to get us down under our limit.
   if (sCurrentDecodedImageBytes > sMaxDecodedImageKB * 1024 &&
-      !sDiscardableImages.isEmpty() && !sDiscardRunnablePending) {
-    sDiscardRunnablePending = true;
-    nsRefPtr<DiscardRunnable> runnable = new DiscardRunnable();
-    NS_DispatchToCurrentThread(runnable);
+      !sDiscardableImages.isEmpty()) {
+    // Check if the value of sDiscardRunnablePending used to be false
+    if (!PR_ATOMIC_SET(&sDiscardRunnablePending, 1)) {
+      nsRefPtr<DiscardRunnable> runnable = new DiscardRunnable();
+      NS_DispatchToMainThread(runnable);
+    }
   }
 }
 
 } // namespace image
 } // namespace mozilla
--- a/image/src/DiscardTracker.h
+++ b/image/src/DiscardTracker.h
@@ -43,42 +43,49 @@ class DiscardTracker
     struct Node : public LinkedListElement<Node>
     {
       RasterImage *img;
       TimeStamp timestamp;
     };
 
     /**
      * Add an image to the front of the tracker's list, or move it to the front
-     * if it's already in the list.
+     * if it's already in the list.  This function is main thread only.
      */
     static nsresult Reset(struct Node* node);
 
     /**
      * Remove a node from the tracker; do nothing if the node is currently
-     * untracked.
+     * untracked.  This function is main thread only.
      */
     static void Remove(struct Node* node);
 
     /**
+     * Initializes the discard tracker.  This function is main thread only.
+     */
+    static nsresult Initialize();
+
+    /**
      * Shut the discard tracker down.  This should be called on XPCOM shutdown
-     * so we destroy the discard timer's nsITimer.
+     * so we destroy the discard timer's nsITimer.  This function is main thread
+     * only.
      */
     static void Shutdown();
 
     /**
      * Discard the decoded image data for all images tracked by the discard
-     * tracker.
+     * tracker.  This function is main thread only.
      */
     static void DiscardAll();
 
     /**
      * Inform the discard tracker that we've allocated or deallocated some
      * memory for a decoded image.  We use this to determine when we've
-     * allocated too much memory and should discard some images.
+     * allocated too much memory and should discard some images.  This function
+     * can be called from any thread and is thread-safe.
      */
     static void InformAllocation(PRInt64 bytes);
 
   private:
     /**
      * This is called when the discard timer fires; it calls into DiscardNow().
      */
     friend int DiscardTimeoutChangedCallback(const char* aPref, void *aClosure);
@@ -87,30 +94,31 @@ class DiscardTracker
      * When run, this runnable sets sDiscardRunnablePending to false and calls
      * DiscardNow().
      */
     class DiscardRunnable : public nsRunnable
     {
       NS_IMETHOD Run();
     };
 
-    static nsresult Initialize();
     static void ReloadTimeout();
     static nsresult EnableTimer();
     static void DisableTimer();
     static void MaybeDiscardSoon();
     static void TimerCallback(nsITimer *aTimer, void *aClosure);
     static void DiscardNow();
 
     static LinkedList<Node> sDiscardableImages;
     static nsCOMPtr<nsITimer> sTimer;
     static bool sInitialized;
     static bool sTimerOn;
-    static bool sDiscardRunnablePending;
+    static PRInt32 sDiscardRunnablePending;
     static PRInt64 sCurrentDecodedImageBytes;
     static PRUint32 sMinDiscardTimeoutMs;
     static PRUint32 sMaxDecodedImageKB;
+    // Lock for safegarding the 64-bit sCurrentDecodedImageBytes
+    static PRLock *sAllocationLock;
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif /* mozilla_imagelib_DiscardTracker_h_ */
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -243,20 +243,19 @@ RasterImage::~RasterImage()
             ("CompressedImageAccounting: destroying RasterImage %p.  "
              "Total Containers: %d, Discardable containers: %d, "
              "Total source bytes: %lld, Source bytes for discardable containers %lld",
              this,
              num_containers,
              num_discardable_containers,
              total_source_bytes,
              discardable_source_bytes));
+    DiscardTracker::Remove(&mDiscardTrackerNode);
   }
 
-  DiscardTracker::Remove(&mDiscardTrackerNode);
-
   // If we have a decoder open, shut it down
   if (mDecoder) {
     nsresult rv = ShutdownDecoder(eShutdownIntent_Interrupted);
     if (NS_FAILED(rv))
       NS_WARNING("Failed to shut down decoder in destructor!");
   }
 
   // Total statistics