Bug 1390856 - Fix XDR helper thread error handling. r=nbp a=gchang
authorTed Campbell <tcampbell@mozilla.com>
Sun, 19 Nov 2017 00:41:32 -0500
changeset 444847 29d36b507287ce7f7c5f1a43bc80d0e311cff0b1
parent 444846 f49b7adf88e63d13a9c7d05630c102eb69d4c24d
child 444848 c818d9deb4627045e57217ed3274697496edff92
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnbp, gchang
bugs1390856
milestone58.0
Bug 1390856 - Fix XDR helper thread error handling. r=nbp a=gchang Make sure not to return TranscodeResult_Ok if an OOM happens while running XDR on a helper thread. Also fix OOM handling when computing BuildID. MozReview-Commit-ID: EPYNhFsclVG
js/src/jit-test/tests/xdr/bug1390856.js
js/src/jsscript.cpp
js/src/vm/Xdr.cpp
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/xdr/bug1390856.js
@@ -0,0 +1,30 @@
+if (!('oomTest' in this))
+    quit();
+
+if (helperThreadCount() == 0)
+    quit();
+
+
+let THREAD_TYPE_PARSE = 4;
+
+// Test main thread encode/decode OOM
+oomTest(function() {
+    let t = cacheEntry(`function f() { function g() { }; return 3; };`);
+
+    evaluate(t, { sourceIsLazy: true, saveIncrementalBytecode: true });
+    evaluate(t, { sourceIsLazy: true, readBytecode: true });
+});
+
+// Test helper thread decode OOM
+let t = cacheEntry(`function f() { function g() { }; return 3; };`);
+evaluate(t, { sourceIsLazy: true, saveIncrementalBytecode: true });
+for (var i = 1; i < 100; ++i) {
+    try {
+        oomAtAllocation(i, THREAD_TYPE_PARSE);
+        offThreadDecodeScript(t);
+        runOffThreadDecodedScript();
+    }
+    catch (e) {
+        assertEq(e, "out of memory");
+    }
+}
--- a/js/src/jsscript.cpp
+++ b/js/src/jsscript.cpp
@@ -2093,38 +2093,46 @@ ScriptSource::xdrEncodeTopLevel(JSContex
 
     if (!xdrEncoder_->init()) {
         ReportOutOfMemory(cx);
         return false;
     }
 
     RootedScript s(cx, script);
     if (!xdrEncoder_->codeScript(&s)) {
-        if (xdrEncoder_->resultCode() == JS::TranscodeResult_Throw)
-            return false;
-        // Encoding failures are reported by the xdrFinalizeEncoder function.
-        return true;
+        // On encoding failure, let failureCase destroy encoder and return true
+        // to avoid failing any currently executing script.
+        if (xdrEncoder_->resultCode() & JS::TranscodeResult_Failure)
+            return true;
+
+        return false;
     }
 
     failureCase.release();
     return true;
 }
 
 bool
 ScriptSource::xdrEncodeFunction(JSContext* cx, HandleFunction fun, HandleScriptSource sourceObject)
 {
     MOZ_ASSERT(sourceObject->source() == this);
     MOZ_ASSERT(hasEncoder());
     auto failureCase = mozilla::MakeScopeExit([&] {
         xdrEncoder_.reset(nullptr);
     });
 
     RootedFunction f(cx, fun);
-    if (!xdrEncoder_->codeFunction(&f, sourceObject))
+    if (!xdrEncoder_->codeFunction(&f, sourceObject)) {
+        // On encoding failure, let failureCase destroy encoder and return true
+        // to avoid failing any currently executing script.
+        if (xdrEncoder_->resultCode() & JS::TranscodeResult_Failure)
+            return true;
+
         return false;
+    }
 
     failureCase.release();
     return true;
 }
 
 bool
 ScriptSource::xdrFinalizeEncoder(JS::TranscodeBuffer& buffer)
 {
--- a/js/src/vm/Xdr.cpp
+++ b/js/src/vm/Xdr.cpp
@@ -26,21 +26,24 @@ LifoAlloc&
 XDRState<mode>::lifoAlloc() const {
     return buf.cx()->tempLifoAlloc();
 }
 
 template<XDRMode mode>
 void
 XDRState<mode>::postProcessContextErrors(JSContext* cx)
 {
-    if (!cx->helperThread() && cx->isExceptionPending()) {
-        MOZ_ASSERT(resultCode_ == JS::TranscodeResult_Ok ||
-                   resultCode_ == JS::TranscodeResult_Throw);
+    // NOTE: This should only be called on transcode failure. Not all failure
+    // paths call XDRState::fail(...), so we should update resultCode_ if it
+    // doesn't hold a specific transcode error.
+
+    if (resultCode_ & JS::TranscodeResult_Failure)
+        MOZ_ASSERT_IF(!cx->helperThread(), !cx->isExceptionPending());
+    else
         resultCode_ = JS::TranscodeResult_Throw;
-    }
 }
 
 template<XDRMode mode>
 bool
 XDRState<mode>::codeChars(const Latin1Char* chars, size_t nchars)
 {
     static_assert(sizeof(Latin1Char) == sizeof(uint8_t), "Latin1Char must fit in 1 byte");
 
@@ -75,18 +78,21 @@ XDRState<mode>::codeChars(char16_t* char
     return true;
 }
 
 template<XDRMode mode>
 static bool
 VersionCheck(XDRState<mode>* xdr)
 {
     JS::BuildIdCharVector buildId;
-    if (!xdr->cx()->buildIdOp() || !xdr->cx()->buildIdOp()(&buildId))
-        return xdr->fail(JS::TranscodeResult_Failure_BadBuildId);
+    MOZ_ASSERT(xdr->cx()->buildIdOp());
+    if (!xdr->cx()->buildIdOp()(&buildId)) {
+        ReportOutOfMemory(xdr->cx());
+        return xdr->fail(JS::TranscodeResult_Throw);
+    }
     MOZ_ASSERT(!buildId.empty());
 
     uint32_t buildIdLength;
     if (mode == XDR_ENCODE)
         buildIdLength = buildId.length();
 
     if (!xdr->codeUint32(&buildIdLength))
         return false;