Bug 1404741: Don't call mozJSComponentLoader::CompilationScope during URLPreloader critical section. r=mccr8
authorKris Maglione <maglione.k@gmail.com>
Fri, 06 Oct 2017 15:09:11 -0700
changeset 385031 a5ab6b153cccc38a2fae62a529923f8370734c39
parent 385030 b632d01bb67047bcdc46787d00ee832b0c4fca27
child 385032 21f762390e63416f410c54da438d4b93de355882
push id32640
push userarchaeopteryx@coole-files.de
push dateSun, 08 Oct 2017 09:44:48 +0000
treeherdermozilla-central@8dba4037f395 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8
bugs1404741
milestone58.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 1404741: Don't call mozJSComponentLoader::CompilationScope during URLPreloader critical section. r=mccr8 The URLPreloader's initialization code accesses the Omnijar cache off-main thread. It can do so safely only as long as the main thread is blocked, or running code which is guaranteed never to touch Omnijar, while its critical section runs. While that was guaranteed for previous versions of the code, some invariants changed when we began using the component loader's shared global for initial compilation. Once the global is created, we can safely use it to initialize compilation. But creating the global invokes a large amount of Gecko code, some of which touches Omnijar in the process. MozReview-Commit-ID: 48WoIZtGPTo
js/xpconnect/loader/ScriptPreloader.cpp
js/xpconnect/loader/ScriptPreloader.h
--- a/js/xpconnect/loader/ScriptPreloader.cpp
+++ b/js/xpconnect/loader/ScriptPreloader.cpp
@@ -411,23 +411,28 @@ ScriptPreloader::InitCache(const nsAStri
     mBaseName = basePath;
 
     RegisterWeakMemoryReporter(this);
 
     if (!XRE_IsParentProcess()) {
         return Ok();
     }
 
+    // Grab the compilation scope before initializing the URLPreloader, since
+    // it's not safe to run component loader code during its critical section.
+    AutoSafeJSAPI jsapi;
+    JS::RootedObject scope(jsapi.cx(), CompilationScope(jsapi.cx()));
+
     // Note: Code on the main thread *must not access Omnijar in any way* until
     // this AutoBeginReading guard is destroyed.
     URLPreloader::AutoBeginReading abr;
 
     MOZ_TRY(OpenCache());
 
-    return InitCacheInternal();
+    return InitCacheInternal(scope);
 }
 
 Result<Ok, nsresult>
 ScriptPreloader::InitCache(const Maybe<ipc::FileDescriptor>& cacheFile, ScriptCacheChild* cacheChild)
 {
     MOZ_ASSERT(XRE_IsContentProcess());
 
     mCacheInitialized = true;
@@ -440,17 +445,17 @@ ScriptPreloader::InitCache(const Maybe<i
     }
 
     MOZ_TRY(mCacheData.init(cacheFile.ref()));
 
     return InitCacheInternal();
 }
 
 Result<Ok, nsresult>
-ScriptPreloader::InitCacheInternal()
+ScriptPreloader::InitCacheInternal(JS::HandleObject scope)
 {
     auto size = mCacheData.size();
 
     uint32_t headerSize;
     if (size < sizeof(MAGIC) + sizeof(headerSize)) {
         return Err(NS_ERROR_UNEXPECTED);
     }
 
@@ -515,17 +520,17 @@ ScriptPreloader::InitCacheInternal()
         if (buf.error()) {
             return Err(NS_ERROR_UNEXPECTED);
         }
 
         mPendingScripts = Move(scripts);
         cleanup.release();
     }
 
-    DecodeNextBatch(OFF_THREAD_FIRST_CHUNK_SIZE);
+    DecodeNextBatch(OFF_THREAD_FIRST_CHUNK_SIZE, scope);
     return Ok();
 }
 
 void
 ScriptPreloader::PrepareCacheWriteInternal()
 {
     MOZ_ASSERT(NS_IsMainThread());
 
@@ -950,17 +955,17 @@ ScriptPreloader::MaybeFinishOffThreadDec
         LOG(Debug, "Finished off-thread decode of %s\n", script->mURL.get());
         if (i < jsScripts.length())
             script->mScript = jsScripts[i++];
         script->mReadyToExecute = true;
     }
 }
 
 void
-ScriptPreloader::DecodeNextBatch(size_t chunkSize)
+ScriptPreloader::DecodeNextBatch(size_t chunkSize, JS::HandleObject scope)
 {
     MOZ_ASSERT(mParsingSources.length() == 0);
     MOZ_ASSERT(mParsingScripts.length() == 0);
 
     auto cleanup = MakeScopeExit([&] () {
         mParsingScripts.clearAndFree();
         mParsingSources.clearAndFree();
     });
@@ -998,17 +1003,17 @@ ScriptPreloader::DecodeNextBatch(size_t 
     }
 
     if (size == 0 && mPendingScripts.isEmpty()) {
         return;
     }
 
     AutoSafeJSAPI jsapi;
     JSContext* cx = jsapi.cx();
-    JSAutoCompartment ac(cx, CompilationScope(cx));
+    JSAutoCompartment ac(cx, scope ? scope : CompilationScope(cx));
 
     JS::CompileOptions options(cx, JSVERSION_DEFAULT);
     options.setNoScriptRval(true)
            .setSourceIsLazy(true);
 
     if (!JS::CanCompileOffThread(cx, options, size) ||
         !JS::DecodeMultiOffThreadScripts(cx, options, mParsingSources,
                                          OffThreadDecodeCallback,
--- a/js/xpconnect/loader/ScriptPreloader.h
+++ b/js/xpconnect/loader/ScriptPreloader.h
@@ -92,17 +92,17 @@ public:
     Result<Ok, nsresult> InitCache(const Maybe<ipc::FileDescriptor>& cacheFile, ScriptCacheChild* cacheChild);
 
     bool Active()
     {
       return mCacheInitialized && !mStartupFinished;
     }
 
 private:
-    Result<Ok, nsresult> InitCacheInternal();
+    Result<Ok, nsresult> InitCacheInternal(JS::HandleObject scope = nullptr);
 
 public:
     void Trace(JSTracer* trc);
 
     static ProcessType CurrentProcessType()
     {
         return sProcessType;
     }
@@ -383,17 +383,17 @@ private:
     // 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);
 
-    void DecodeNextBatch(size_t chunkSize);
+    void DecodeNextBatch(size_t chunkSize, JS::HandleObject scope = nullptr);
 
     static void OffThreadDecodeCallback(void* token, void* context);
     void MaybeFinishOffThreadDecode();
     void DoFinishOffThreadDecode();
 
     // Returns the global scope object for off-thread compilation. When global
     // sharing is enabled in the component loader, this should be the shared
     // module global. Otherwise, it should be the XPConnect compilation scope.