Bug 1390856 - Fix XDR helper thread error handling. r?nbp draft
authorTed Campbell <tcampbell@mozilla.com>
Sun, 19 Nov 2017 00:41:32 -0500
changeset 700653 ce556fee5953a08df1116394a39f3767bfe74e45
parent 700226 b056526be38e96b3e381b7e90cd8254ad1d96d9d
child 740955 a6c45f032c99fc374579e8b28aed798d33b43e6e
push id89923
push userbmo:tcampbell@mozilla.com
push dateMon, 20 Nov 2017 17:36:59 +0000
reviewersnbp
bugs1390856
milestone59.0a1
Bug 1390856 - Fix XDR helper thread error handling. r?nbp 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
@@ -2085,38 +2085,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;