Bug 1371248: Avoid hangs when preloader cache flush is triggered during shutdown. r=erahm
☠☠ backed out by 4bd7d5752bd8 ☠ ☠
authorKris Maglione <maglione.k@gmail.com>
Wed, 28 Jun 2017 14:46:30 -0700
changeset 367149 7e2f1f4713fcf4ad54559f7f1fe6cacc30d32512
parent 367148 6563cbe90004deb454f7519835cf425334f3b4e2
child 367150 8a93cfede99e13c829553af5bc1cb3c23911886e
push id45872
push usermaglione.k@gmail.com
push dateMon, 03 Jul 2017 21:04:32 +0000
treeherderautoland@7e2f1f4713fc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm
bugs1371248
milestone56.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 1371248: Avoid hangs when preloader cache flush is triggered during shutdown. r=erahm 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.
+        PrepareCacheWriteInternal();
+
         // 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());
 
+    mSaveMonitor.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(mSaveMonitor);
+
+    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,25 +585,30 @@ 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("ScriptPreloader::PrepareCacheWrite",
                             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")));
 
@@ -637,18 +664,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;