Bug 1371248 - Avoid hangs when preloader cache flush is triggered during shutdown. r=erahm, a=jcristau
authorKris Maglione <maglione.k@gmail.com>
Wed, 28 Jun 2017 14:46:30 -0700
changeset 411836 477e9cf4749380b01d6335f8068013b56a65aaa0
parent 411835 58a8777e40a7f153aa9d8ab7b05e9c9e5921f85c
child 411837 6a8f9e8bcabd271f6c762ab49535798eaf6b84a2
push id7479
push userryanvm@gmail.com
push dateThu, 06 Jul 2017 15:51:35 +0000
treeherdermozilla-beta@1ae7e516e5da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm, jcristau
bugs1371248
milestone55.0
Bug 1371248 - Avoid hangs when preloader cache flush is triggered during shutdown. r=erahm, a=jcristau MozReview-Commit-ID: FpW53d5TTCG
js/xpconnect/loader/ScriptPreloader.cpp
js/xpconnect/loader/ScriptPreloader.h
--- a/js/xpconnect/loader/ScriptPreloader.cpp
+++ b/js/xpconnect/loader/ScriptPreloader.cpp
@@ -236,28 +236,36 @@ ScriptPreloader::ScriptPreloader()
 }
 
 void
 ScriptPreloader::ForceWriteCacheFile()
 {
     if (mSaveThread) {
         MonitorAutoLock mal(mSaveMonitor);
 
+        // Make sure we've prepared scripts, so we don't risk deadlocking while
+        // dispatching the prepare task during shutdown.
+        PrepareCacheWrite();
+
         // Unblock the save thread, so it can start saving before we get to
         // XPCOM shutdown.
         mal.Notify();
     }
 }
 
 void
 ScriptPreloader::Cleanup()
 {
     if (mSaveThread) {
         MonitorAutoLock mal(mSaveMonitor);
 
+        // Make sure the save thread is not blocked dispatching a sync task to
+        // the main thread, or we will deadlock.
+        MOZ_RELEASE_ASSERT(!mBlockedOnSyncDispatch);
+
         while (!mSaveComplete && mSaveThread) {
             mal.Wait();
         }
     }
 
     mScripts.Clear();
 
     AutoSafeJSAPI jsapi;
@@ -283,16 +291,20 @@ ScriptPreloader::InvalidateCache()
     // If we've already finished saving the cache at this point, start a new
     // delayed save operation. This will write out an empty cache file in place
     // of any cache file we've already written out this session, which will
     // prevent us from falling back to the current session's cache file on the
     // next startup.
     if (mSaveComplete && mChildCache) {
         mSaveComplete = false;
 
+        // Make sure scripts are prepared to avoid deadlock when invalidating
+        // the cache during shutdown.
+        PrepareCacheWriteInternal();
+
         Unused << NS_NewNamedThread("SaveScripts",
                                     getter_AddRefs(mSaveThread), this);
     }
 }
 
 nsresult
 ScriptPreloader::Observe(nsISupports* subject, const char* topic, const char16_t* data)
 {
@@ -494,20 +506,22 @@ Write(PRFileDesc* fd, const void* data, 
 {
     if (PR_Write(fd, data, len) != len) {
         return Err(NS_ERROR_FAILURE);
     }
     return Ok();
 }
 
 void
-ScriptPreloader::PrepareCacheWrite()
+ScriptPreloader::PrepareCacheWriteInternal()
 {
     MOZ_ASSERT(NS_IsMainThread());
 
+    mMonitor.AssertCurrentThreadOwns();
+
     auto cleanup = MakeScopeExit([&] () {
         if (mChildCache) {
             mChildCache->PrepareCacheWrite();
         }
     });
 
     if (mDataPrepared) {
         return;
@@ -542,16 +556,24 @@ ScriptPreloader::PrepareCacheWrite()
     if (!found) {
         mSaveComplete = true;
         return;
     }
 
     mDataPrepared = true;
 }
 
+void
+ScriptPreloader::PrepareCacheWrite()
+{
+    MonitorAutoLock mal(mMonitor);
+
+    PrepareCacheWriteInternal();
+}
+
 // Writes out a script cache file for the scripts accessed during early
 // startup in this session. The cache file is a little-endian binary file with
 // the following format:
 //
 // - A uint32 containing the size of the header block.
 //
 // - A header entry for each file stored in the cache containing:
 //   - The URL that the script was originally read from.
@@ -563,23 +585,28 @@ ScriptPreloader::PrepareCacheWrite()
 // - A block of XDR data for the encoded scripts, with each script's data at
 //   an offset from the start of the block, as specified above.
 Result<Ok, nsresult>
 ScriptPreloader::WriteCache()
 {
     MOZ_ASSERT(!NS_IsMainThread());
 
     if (!mDataPrepared && !mSaveComplete) {
+        MOZ_ASSERT(!mBlockedOnSyncDispatch);
+        mBlockedOnSyncDispatch = true;
+
         MonitorAutoUnlock mau(mSaveMonitor);
 
         NS_DispatchToMainThread(
             NewRunnableMethod(this, &ScriptPreloader::PrepareCacheWrite),
             NS_DISPATCH_SYNC);
     }
 
+    mBlockedOnSyncDispatch = false;
+
     if (mSaveComplete) {
         // If we don't have anything we need to save, we're done.
         return Ok();
     }
 
     nsCOMPtr<nsIFile> cacheFile;
     MOZ_TRY_VAR(cacheFile, GetCacheFile(NS_LITERAL_STRING("-new.bin")));
 
@@ -635,18 +662,22 @@ ScriptPreloader::WriteCache()
 // Runs in the mSaveThread thread, and writes out the cache file for the next
 // session after a reasonable delay.
 nsresult
 ScriptPreloader::Run()
 {
     MonitorAutoLock mal(mSaveMonitor);
 
     // Ideally wait about 10 seconds before saving, to avoid unnecessary IO
-    // during early startup.
-    mal.Wait(10000);
+    // during early startup. But only if the cache hasn't been invalidated,
+    // since that can trigger a new write during shutdown, and we don't want to
+    // cause shutdown hangs.
+    if (!mCacheInvalidated) {
+        mal.Wait(10000);
+    }
 
     auto result = WriteCache();
     Unused << NS_WARN_IF(result.isErr());
 
     result = mChildCache->WriteCache();
     Unused << NS_WARN_IF(result.isErr());
 
     mSaveComplete = true;
--- a/js/xpconnect/loader/ScriptPreloader.h
+++ b/js/xpconnect/loader/ScriptPreloader.h
@@ -362,16 +362,18 @@ private:
 
     // Writes a new cache file to disk. Must not be called on the main thread.
     Result<Ok, nsresult> WriteCache();
 
     // Prepares scripts for writing to the cache, serializing new scripts to
     // XDR, and calculating their size-based offsets.
     void PrepareCacheWrite();
 
+    void PrepareCacheWriteInternal();
+
     // Returns a file pointer for the cache file with the given name in the
     // current profile.
     Result<nsCOMPtr<nsIFile>, nsresult>
     GetCacheFile(const nsAString& suffix);
 
     // Waits for the given cached script to finish compiling off-thread, or
     // decodes it synchronously on the main thread, as appropriate.
     JSScript* WaitForCachedScript(JSContext* cx, CachedScript* script);
@@ -405,16 +407,17 @@ private:
     // True after we've shown the first window, and are no longer adding new
     // scripts to the cache.
     bool mStartupFinished = false;
 
     bool mCacheInitialized = false;
     bool mSaveComplete = false;
     bool mDataPrepared = false;
     bool mCacheInvalidated = false;
+    bool mBlockedOnSyncDispatch = false;
 
     // The list of scripts that we read from the initial startup cache file,
     // but have yet to initiate a decode task for.
     LinkedList<CachedScript> mPendingScripts;
 
     // The lists of scripts and their sources that make up the chunk currently
     // being decoded in a background thread.
     JS::TranscodeSources mParsingSources;