Bug 1404741 - Don't call mozJSComponentLoader::CompilationScope during URLPreloader critical section. r=mccr8, a=ritu
authorKris Maglione <maglione.k@gmail.com>
Fri, 06 Oct 2017 15:09:11 -0700
changeset 432316 c953cae869d8
parent 432315 4499acb9b54b
child 432317 c4235f5a700c
push id7929
push userryanvm@gmail.com
push date2017-10-09 18:47 +0000
treeherdermozilla-beta@c4235f5a700c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmccr8, ritu
bugs1404741
milestone57.0
Bug 1404741 - Don't call mozJSComponentLoader::CompilationScope during URLPreloader critical section. r=mccr8, a=ritu 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.