Bug 472383 - 'Workers: JS/C++/JS/C++ recursion not properly guarded'. r+sr=jst, a=blocking1.9.1+

dom/src/threads/nsDOMThreadService.cpp

@@ -60,6 +60,7 @@
 #include "nsPIDOMWindow.h"
 
 // Other includes
+#include "jscntxt.h"
 #include "nsAutoLock.h"
 #include "nsAutoPtr.h"
 #include "nsContentUtils.h"
@@ -488,6 +489,11 @@ DOMWorkerErrorReporter(JSContext* aCx,
     return;
   }
 
+  if (worker->mErrorHandlerRecursionCount == 2) {
+    // We've somehow ended up in a recursive onerror loop. Bail out.
+    return;
+  }
+
   nsresult rv;
   nsCOMPtr<nsIScriptError> scriptError =
     do_CreateInstance(NS_SCRIPTERROR_CONTRACTID, &rv);
@@ -508,27 +514,36 @@ DOMWorkerErrorReporter(JSContext* aCx,
                          column, aReport->flags, "DOM Worker javascript");
   NS_ENSURE_SUCCESS(rv,);
 
-  // Try the onerror handler for the worker's scope.
-  nsCOMPtr<nsIDOMEventListener> handler =
-    worker->mInnerHandler->GetOnXListener(NS_LITERAL_STRING("error"));
+  // Don't call the error handler if we're out of stack space.
+  if (aReport->errorNumber != JSMSG_SCRIPT_STACK_QUOTA &&
+      aReport->errorNumber != JSMSG_OVER_RECURSED) {
+    // Try the onerror handler for the worker's scope.
+    nsCOMPtr<nsIDOMEventListener> handler =
+      worker->mInnerHandler->GetOnXListener(NS_LITERAL_STRING("error"));
 
-  if (handler) {
-    nsRefPtr<nsDOMWorkerErrorEvent> event(new nsDOMWorkerErrorEvent());
-    NS_ENSURE_TRUE(event,);
+    if (handler) {
+      nsRefPtr<nsDOMWorkerErrorEvent> event(new nsDOMWorkerErrorEvent());
+      if (event) {
+        rv = event->InitErrorEvent(NS_LITERAL_STRING("error"), PR_FALSE, PR_TRUE,
+                                   nsDependentString(message), filename,
+                                   aReport->lineno);
+        if (NS_SUCCEEDED(rv)) {
+          NS_ASSERTION(worker->GetInnerScope(), "Null scope!");
+          event->SetTarget(worker->GetInnerScope());
 
-    rv = event->InitErrorEvent(NS_LITERAL_STRING("error"), PR_FALSE, PR_TRUE,
-                               nsDependentString(message), filename,
-                               aReport->lineno);
-    NS_ENSURE_SUCCESS(rv,);
+          NS_ASSERTION(worker->mErrorHandlerRecursionCount >= 0,
+                       "Bad recursion count logic!");
+          worker->mErrorHandlerRecursionCount++;
 
-    NS_ASSERTION(worker->GetInnerScope(), "Null scope!");
-    event->SetTarget(worker->GetInnerScope());
+          handler->HandleEvent(static_cast<nsDOMWorkerEvent*>(event));
 
-    rv = handler->HandleEvent(static_cast<nsDOMWorkerEvent*>(event));
-    NS_ENSURE_SUCCESS(rv,);
+          worker->mErrorHandlerRecursionCount--;
 
-    if (event->PreventDefaultCalled()) {
-      return;
+          if (event->PreventDefaultCalled()) {
+            return;
+          }
+        }
+      }
     }
   }
 
@@ -838,6 +853,25 @@ nsDOMThreadService::CreateJSContext()
     SetSecurityManagerForJSContext(cx, gWorkerSecurityManager, 0);
   NS_ENSURE_SUCCESS(rv, nsnull);
 
+  PRUint32 stackDummy;
+  jsuword stackLimit, currentStackAddr = (jsuword)&stackDummy;
+
+  // 256k stack space.
+  const jsuword kStackSize = 0x40000;
+
+#if JS_STACK_GROWTH_DIRECTION < 0
+  stackLimit = (currentStackAddr > kStackSize) ?
+               currentStackAddr - kStackSize :
+               0;
+#else
+  stackLimit = (currentStackAddr + kStackSize > currentStackAddr) ?
+               currentStackAddr + kStackSize :
+               (jsuword) -1;
+#endif
+
+  JS_SetThreadStackLimit(cx, stackLimit);
+  JS_SetScriptStackQuota(cx, 100*1024*1024);
+
   return cx.forget();
 }
 

dom/src/threads/nsDOMWorker.cpp

@@ -902,6 +902,7 @@ nsDOMWorker::nsDOMWorker(nsDOMWorker* aParent,
   mNextTimeoutId(0),
   mFeatureSuspendDepth(0),
   mWrappedNative(nsnull),
+  mErrorHandlerRecursionCount(0),
   mCanceled(PR_FALSE),
   mSuspended(PR_FALSE),
   mCompileAttempted(PR_FALSE),

dom/src/threads/nsDOMWorker.h

@@ -238,6 +238,8 @@ private:
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsCOMPtr<nsIURI> mURI;
 
+  PRInt32 mErrorHandlerRecursionCount;
+
   PRPackedBool mCanceled;
   PRPackedBool mSuspended;
   PRPackedBool mCompileAttempted;

dom/src/threads/test/Makefile.in

@@ -84,6 +84,8 @@ _TEST_FILES = \
   threadErrors_worker4.js \
   test_threadTimeouts.html \
   threadTimeouts_worker.js \
+  test_throwingOnerror.html \
+  throwingOnerror_worker.js \
   test_xhr.html \
   xhr_worker.js \
   test_xhrAbort.html \

dom/src/threads/test/recursion_worker.js

@@ -1,14 +1,34 @@
+onerror = function(event) {
+  // Do nothing.
+};
+
+// Pure JS recursion
 function recurse() {
   recurse();
 }
 
-onmessage = function(event) {
-  switch (event.data) {
-    case "start":
-      recurse();
-      throw "Never should have gotten here!";
-      break;
-    default:
-      throw "Bad message: " + event.data;
+// JS -> C++ -> JS -> C++ recursion
+function recurse2() {
+  var xhr = new XMLHttpRequest();
+  xhr.onreadystatechange = function() {
+    xhr.open("GET", "nonexistent.file");
   }
+  xhr.open("GET", "nonexistent.file");
+  postMessage("Done");
+}
+
+var count = 0;
+onmessage = function(event) {
+  switch (++count) {
+    case 1:
+      recurse();
+      break;
+    case 2:
+      recurse2();
+      // Have to return here because we don't propagate exceptions from event
+      // handlers!
+      return;
+    default:
+  }
+  throw "Never should have gotten here!";
 }

dom/src/threads/test/test_recursion.html

@@ -17,14 +17,22 @@ Tests of DOM Worker Threads
 <pre id="test">
 <script class="testbody" type="text/javascript">
 
+  const testCount = 2;
+
   var worker = new Worker("recursion_worker.js");
 
-  worker.onerror = function(event) {
-    is(event.message, "too much recursion");
+  worker.onmessage = function(event) {
+    is(event.data, "Done");
     SimpleTest.finish();
   }
 
-  worker.postMessage("start");
+  worker.onerror = function(event) {
+    is(event.message, "too much recursion");
+  }
+
+  for (var i = 0; i < testCount; i++) {
+    worker.postMessage("start");
+  }
 
   SimpleTest.waitForExplicitFinish();
 

dom/src/threads/test/test_throwingOnerror.html

@@ -0,0 +1,50 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+Tests of DOM Worker Threads
+-->
+<head>
+  <title>Test for DOM Worker Threads Recursion</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+<p id="display"></p>
+<div id="content" style="display: none">
+
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+  var worker = new Worker("throwingOnerror_worker.js");
+
+  var errors = ["foo", "bar"];
+
+  worker.onerror = function(event) {
+    var found = false;
+    for (var index in errors) {
+      if (event.message = "uncaught exception: " + errors[index]) {
+        errors.splice(index, 1);
+        found = true;
+        break;
+      }
+    }
+    is(found, true, "Unexpected error!");
+  };
+
+  worker.onmessage = function(event) {
+    is(errors.length, 0, "Didn't see expected errors!");
+    SimpleTest.finish();
+  };
+
+  for (var i = 0; i < 2; i++) {
+    worker.postMessage("");
+  }
+
+  SimpleTest.waitForExplicitFinish();
+
+</script>
+</pre>
+</body>
+</html>

dom/src/threads/test/throwingOnerror_worker.js

@@ -0,0 +1,11 @@
+onerror = function(event) {
+  throw "bar";
+};
+
+var count = 0;
+onmessage = function(event) {
+  if (!count++) {
+    throw "foo";
+  }
+  postMessage("");
+};