Bug 1404741: Don't call mozJSComponentLoader::CompilationScope during URLPreloader critical section. r?mccr8 draft
authorKris Maglione <maglione.k@gmail.com>
Fri, 06 Oct 2017 15:09:11 -0700
changeset 676251 e8ea9e68e7f384eee73d8e046dfa92e38adaa41c
parent 676250 ccb09cd814661a1beebb22f6e4034eb2a6bfbf40
child 734894 ba1f128affbe87c944dd989442fe7631fea409bb
push id83446
push usermaglione.k@gmail.com
push dateFri, 06 Oct 2017 22:14:09 +0000
reviewersmccr8
bugs1404741
milestone58.0a1
Bug 1404741: Don't call mozJSComponentLoader::CompilationScope during URLPreloader critical section. r?mccr8 The URLPreloader's initialization code access 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, 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.