Bug 1259129 - Fix wrong CondVar::Wait() usage in netwerk/cache, r=honzab
authorMichal Novotny <michal.novotny@gmail.com>
Fri, 13 May 2016 08:56:22 +0200
changeset 297342 f45c25b6a21de74492c2f954ebf1b093b5f58154
parent 297341 74fcb8f55064c67275f77e8d29a8a9d10383db05
child 297343 4f43207abcb992d1604c8047c2cab3f3e6db7966
push id19218
push userkwierso@gmail.com
push dateFri, 13 May 2016 23:46:15 +0000
treeherderfx-team@93d60e9db618 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershonzab
bugs1259129
milestone49.0a1
Bug 1259129 - Fix wrong CondVar::Wait() usage in netwerk/cache, r=honzab
netwerk/cache/nsCacheService.cpp
netwerk/cache/nsCacheService.h
netwerk/cache/nsCacheUtils.cpp
netwerk/cache/nsCacheUtils.h
netwerk/cache/nsDeleteDir.cpp
netwerk/cache/nsDeleteDir.h
--- a/netwerk/cache/nsCacheService.cpp
+++ b/netwerk/cache/nsCacheService.cpp
@@ -291,16 +291,17 @@ class nsBlockOnCacheThreadEvent : public
 public:
     nsBlockOnCacheThreadEvent()
     {
     }
     NS_IMETHOD Run()
     {
         nsCacheServiceAutoLock autoLock(LOCK_TELEM(NSBLOCKONCACHETHREADEVENT_RUN));
         CACHE_LOG_DEBUG(("nsBlockOnCacheThreadEvent [%p]\n", this));
+        nsCacheService::gService->mNotified = true;
         nsCacheService::gService->mCondVar.Notify();
         return NS_OK;
     }
 };
 
 
 nsresult
 nsCacheProfilePrefObserver::Install()
@@ -847,19 +848,22 @@ nsCacheService::SyncWithCacheIOThread()
     nsresult rv =
         gService->mCacheIOThread->Dispatch(event, NS_DISPATCH_NORMAL);
     if (NS_FAILED(rv)) {
         NS_WARNING("Failed dispatching block-event");
         return NS_ERROR_UNEXPECTED;
     }
 
     // wait until notified, then return
-    rv = gService->mCondVar.Wait();
-
-    return rv;
+    gService->mNotified = false;
+    while (!gService->mNotified) {
+      gService->mCondVar.Wait();
+    }
+
+    return NS_OK;
 }
 
 
 bool
 nsCacheProfilePrefObserver::DiskCacheEnabled()
 {
     if ((mDiskCacheCapacity == 0) || (!mDiskCacheParentDirectory))  return false;
     return mDiskCacheEnabled && (!mSanitizeOnShutdown || !mClearCacheOnShutdown);
@@ -1068,16 +1072,17 @@ nsCacheService *   nsCacheService::gServ
 
 NS_IMPL_ISUPPORTS(nsCacheService, nsICacheService, nsICacheServiceInternal,
                   nsIMemoryReporter)
 
 nsCacheService::nsCacheService()
     : mObserver(nullptr),
       mLock("nsCacheService.mLock"),
       mCondVar(mLock, "nsCacheService.mCondVar"),
+      mNotified(false),
       mTimeStampLock("nsCacheService.mTimeStampLock"),
       mInitialized(false),
       mClearingEntries(false),
       mEnableMemoryDevice(true),
       mEnableDiskDevice(true),
       mMemoryDevice(nullptr),
       mDiskDevice(nullptr),
       mOfflineDevice(nullptr),
--- a/netwerk/cache/nsCacheService.h
+++ b/netwerk/cache/nsCacheService.h
@@ -322,16 +322,17 @@ private:
     static nsCacheService *         gService;  // there can be only one...
 
     nsCOMPtr<mozIStorageService>    mStorageService;
 
     nsCacheProfilePrefObserver *    mObserver;
 
     mozilla::Mutex                  mLock;
     mozilla::CondVar                mCondVar;
+    bool                            mNotified;
 
     mozilla::Mutex                  mTimeStampLock;
     mozilla::TimeStamp              mLockAcquiredTimeStamp;
 
     nsCOMPtr<nsIThread>             mCacheIOThread;
 
     nsTArray<nsISupports*>          mDoomedObjects;
     nsCOMPtr<nsITimer>              mSmartSizeTimer;
--- a/netwerk/cache/nsCacheUtils.cpp
+++ b/netwerk/cache/nsCacheUtils.cpp
@@ -20,18 +20,18 @@ public:
     mThread->Shutdown();
     return NS_OK;
   }
 private:
   nsCOMPtr<nsIThread> mThread;
 };
 
 nsShutdownThread::nsShutdownThread(nsIThread *aThread)
-  : mLock("nsShutdownThread.mLock")
-  , mCondVar(mLock, "nsShutdownThread.mCondVar")
+  : mMonitor("nsShutdownThread.mMonitor")
+  , mShuttingDown(false)
   , mThread(aThread)
 {
 }
 
 nsShutdownThread::~nsShutdownThread()
 {
 }
 
@@ -57,29 +57,33 @@ nsShutdownThread::BlockingShutdown(nsITh
 
   rv = NS_NewNamedThread("thread shutdown", getter_AddRefs(workerThread));
   if (NS_FAILED(rv)) {
     NS_WARNING("Can't create nsShutDownThread worker thread!");
     return rv;
   }
 
   {
-    MutexAutoLock lock(st->mLock);
+    MonitorAutoLock mon(st->mMonitor);
     rv = workerThread->Dispatch(st, NS_DISPATCH_NORMAL);
     if (NS_FAILED(rv)) {
       NS_WARNING(
         "Dispatching event in nsShutdownThread::BlockingShutdown failed!");
     } else {
-      st->mCondVar.Wait();
+      st->mShuttingDown = true;
+      while (st->mShuttingDown) {
+        mon.Wait();
+      }
     }
   }
 
   return Shutdown(workerThread);
 }
 
 NS_IMETHODIMP
 nsShutdownThread::Run()
 {
-  MutexAutoLock lock(mLock);
+  MonitorAutoLock mon(mMonitor);
   mThread->Shutdown();
-  mCondVar.Notify();
+  mShuttingDown = false;
+  mon.Notify();
   return NS_OK;
 }
--- a/netwerk/cache/nsCacheUtils.h
+++ b/netwerk/cache/nsCacheUtils.h
@@ -4,18 +4,17 @@
  * 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/. */
 
 #ifndef _nsCacheUtils_h_
 #define _nsCacheUtils_h_
 
 #include "nsThreadUtils.h"
 #include "nsCOMPtr.h"
-#include "mozilla/Mutex.h"
-#include "mozilla/CondVar.h"
+#include "mozilla/Monitor.h"
 
 class nsIThread;
 
 /**
  * A class with utility methods for shutting down nsIThreads easily.
   */
 class nsShutdownThread : public mozilla::Runnable {
 public:
@@ -31,14 +30,14 @@ public:
 
 /**
  * BlockingShutdown ensures that by the time it returns, aThread->Shutdown() has
  * been called and no pending events have been processed on the current thread.
  */
   static nsresult BlockingShutdown(nsIThread *aThread);
 
 private:
-  mozilla::Mutex      mLock;
-  mozilla::CondVar    mCondVar;
+  mozilla::Monitor    mMonitor;
+  bool                mShuttingDown;
   nsCOMPtr<nsIThread> mThread;
 };
 
 #endif
--- a/netwerk/cache/nsDeleteDir.cpp
+++ b/netwerk/cache/nsDeleteDir.cpp
@@ -20,27 +20,29 @@
 using namespace mozilla;
 
 class nsBlockOnBackgroundThreadEvent : public Runnable {
 public:
   nsBlockOnBackgroundThreadEvent() {}
   NS_IMETHOD Run()
   {
     MutexAutoLock lock(nsDeleteDir::gInstance->mLock);
+    nsDeleteDir::gInstance->mNotified = true;
     nsDeleteDir::gInstance->mCondVar.Notify();
     return NS_OK;
   }
 };
 
 
 nsDeleteDir * nsDeleteDir::gInstance = nullptr;
 
 nsDeleteDir::nsDeleteDir()
   : mLock("nsDeleteDir.mLock"),
     mCondVar(mLock, "nsDeleteDir.mCondVar"),
+    mNotified(false),
     mShutdownPending(false),
     mStopDeleting(false)
 {
   NS_ASSERTION(gInstance==nullptr, "multiple nsCacheService instances!");
 }
 
 nsDeleteDir::~nsDeleteDir()
 {
@@ -96,17 +98,20 @@ nsDeleteDir::Shutdown(bool finishDeletin
       // has completed all work and can be shutdown
       nsCOMPtr<nsIRunnable> event = new nsBlockOnBackgroundThreadEvent();
       nsresult rv = thread->Dispatch(event, NS_DISPATCH_NORMAL);
       if (NS_FAILED(rv)) {
         NS_WARNING("Failed dispatching block-event");
         return NS_ERROR_UNEXPECTED;
       }
 
-      rv = gInstance->mCondVar.Wait();
+      gInstance->mNotified = false;
+      while (!gInstance->mNotified) {
+        gInstance->mCondVar.Wait();
+      }
       nsShutdownThread::BlockingShutdown(thread);
     }
   }
 
   delete gInstance;
 
   for (int32_t i = 0; i < dirsToRemove.Count(); i++)
     dirsToRemove[i]->Remove(true);
--- a/netwerk/cache/nsDeleteDir.h
+++ b/netwerk/cache/nsDeleteDir.h
@@ -65,15 +65,16 @@ private:
   nsresult InitThread();
   void     DestroyThread();
   nsresult PostTimer(void *arg, uint32_t delay);
   nsresult RemoveDir(nsIFile *file, bool *stopDeleting);
 
   static nsDeleteDir * gInstance;
   mozilla::Mutex       mLock;
   mozilla::CondVar     mCondVar;
+  bool                 mNotified;
   nsCOMArray<nsITimer> mTimers;
   nsCOMPtr<nsIThread>  mThread;
   bool                 mShutdownPending;
   bool                 mStopDeleting;
 };
 
 #endif  // nsDeleteDir_h__