Bug 677273 - 'Crash in mozilla::dom::workers::WorkerPrivate::RescheduleTimeoutTimer'. r=sicking.
authorBen Turner <bent.mozilla@gmail.com>
Fri, 26 Aug 2011 00:34:10 -0700
changeset 77181 ca029820f74064a480b2065092e3863dfe0e49fd
parent 77180 62fd8154f8de12569f080a84d8d58e8e1ca668c0
child 77182 c72e3869695026552cad35f63523fd34e41d387d
push id78
push userclegnitto@mozilla.com
push dateFri, 16 Dec 2011 17:32:24 +0000
treeherdermozilla-release@79d24e644fdd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking
bugs677273
milestone9.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 677273 - 'Crash in mozilla::dom::workers::WorkerPrivate::RescheduleTimeoutTimer'. r=sicking.
dom/workers/WorkerPrivate.cpp
dom/workers/test/close_worker.js
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -3222,105 +3222,104 @@ WorkerPrivate::RunExpiredTimeouts(JSCont
   if (mRunningExpiredTimeouts || !mTimerRunning) {
     return true;
   }
 
   NS_ASSERTION(!mTimeouts.IsEmpty(), "Should have some work to do!");
 
   bool retval = true;
 
-  TimeStamp now = TimeStamp::Now();
+  AutoPtrComparator<TimeoutInfo> comparator = GetAutoPtrComparator(mTimeouts);
+  JSObject* global = JS_GetGlobalObject(aCx);
+  JSPrincipals* principal = GetWorkerPrincipal();
 
   // We want to make sure to run *something*, even if the timer fired a little
   // early. Fudge the value of now to at least include the first timeout.
-  now = NS_MAX(now, mTimeouts[0]->mTargetTime);
+  const TimeStamp now = NS_MAX(TimeStamp::Now(), mTimeouts[0]->mTargetTime);
 
   nsAutoTArray<TimeoutInfo*, 10> expiredTimeouts;
   for (PRUint32 index = 0; index < mTimeouts.Length(); index++) {
     nsAutoPtr<TimeoutInfo>& info = mTimeouts[index];
-    if (info->mTargetTime <= now || info->mCanceled) {
-      expiredTimeouts.AppendElement(info);
+    if (info->mTargetTime > now) {
+      break;
     }
+    expiredTimeouts.AppendElement(info);
   }
 
   // Guard against recursion.
   mRunningExpiredTimeouts = true;
 
   // Run expired timeouts.
   for (PRUint32 index = 0; index < expiredTimeouts.Length(); index++) {
     TimeoutInfo*& info = expiredTimeouts[index];
 
     if (info->mCanceled) {
       continue;
     }
 
-    JSObject* global = JS_GetGlobalObject(aCx);
+    // Always call JS_ReportPendingException if something fails, and if
+    // JS_ReportPendingException returns false (i.e. uncatchable exception) then
+    // break out of the loop.
 
     if (JSVAL_IS_STRING(info->mTimeoutVal)) {
       JSString* expression = JSVAL_TO_STRING(info->mTimeoutVal);
 
       size_t stringLength;
       const jschar* string = JS_GetStringCharsAndLength(aCx, expression,
                                                         &stringLength);
-      if (!string) {
-        if (!JS_ReportPendingException(aCx)) {
-          retval = false;
-          break;
-        }
-        continue;
-      }
-
-      if (!JS_EvaluateUCScriptForPrincipals(aCx, global, GetWorkerPrincipal(),
-                                            string, stringLength,
-                                            info->mFilename.get(),
-                                            info->mLineNumber, nsnull)) {
-        if (!JS_ReportPendingException(aCx)) {
-          retval = false;
-          break;
-        }
-        continue;
+
+      if ((!string ||
+           !JS_EvaluateUCScriptForPrincipals(aCx, global, principal, string,
+                                             stringLength,
+                                             info->mFilename.get(),
+                                             info->mLineNumber, nsnull)) &&
+          !JS_ReportPendingException(aCx)) {
+        retval = false;
+        break;
       }
     }
     else {
       jsval rval;
       if (!JS_CallFunctionValue(aCx, global, info->mTimeoutVal,
                                 info->mExtraArgVals.Length(),
-                                info->mExtraArgVals.Elements(), &rval)) {
-        if (!JS_ReportPendingException(aCx)) {
-          retval = false;
-          break;
-        }
+                                info->mExtraArgVals.Elements(), &rval) &&
+          !JS_ReportPendingException(aCx)) {
+        retval = false;
+        break;
       }
     }
+
+    // Reschedule intervals.
+    if (info->mIsInterval) {
+      PRUint32 timeoutIndex = mTimeouts.IndexOf(info);
+      NS_ASSERTION(timeoutIndex != PRUint32(-1),
+                   "Should still be in the main list!");
+
+      mTimeouts[timeoutIndex].forget();
+      mTimeouts.RemoveElementAt(timeoutIndex);
+
+      NS_ASSERTION(!mTimeouts.Contains(info), "Shouldn't have duplicates!");
+
+      info->mTargetTime += info->mInterval;
+      mTimeouts.InsertElementSorted(info, comparator);
+    }
   }
 
   // No longer possible to be called recursively.
   mRunningExpiredTimeouts = false;
 
-  // Clean up expired and canceled timeouts, reschedule intervals.
-  for (PRUint32 index = 0; index < expiredTimeouts.Length(); index++) {
-    TimeoutInfo*& info = expiredTimeouts[index];
-    if (info->mCanceled || !info->mIsInterval) {
+  // Now remove canceled and expired timeouts from the main list.
+  for (PRUint32 index = 0; index < mTimeouts.Length(); ) {
+    nsAutoPtr<TimeoutInfo>& info = mTimeouts[index];
+    if (info->mTargetTime <= now || info->mCanceled) {
       mTimeouts.RemoveElement(info);
-      continue;
     }
-
-    PRUint32 timeoutIndex = mTimeouts.IndexOf(info);
-    NS_ASSERTION(timeoutIndex != PRUint32(-1),
-                 "Should still be in the other list!");
-
-    mTimeouts[timeoutIndex].forget();
-    mTimeouts.RemoveElementAt(timeoutIndex);
-
-    NS_ASSERTION(!mTimeouts.Contains(info), "Shouldn't have duplicates!");
-
-    AutoPtrComparator<TimeoutInfo> comparator = GetAutoPtrComparator(mTimeouts);
-
-    info->mTargetTime += info->mInterval;
-    mTimeouts.InsertElementSorted(info, comparator);
+    else {
+      index++;
+    }
   }
 
   // Signal the parent that we're no longer using timeouts or reschedule the
   // timer.
   if (mTimeouts.IsEmpty()) {
     if (!ModifyBusyCountFromWorker(aCx, false)) {
       retval = false;
     }
--- a/dom/workers/test/close_worker.js
+++ b/dom/workers/test/close_worker.js
@@ -1,9 +1,14 @@
 /**
  * Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 onclose = function() {
   postMessage("closed");
 };
 
-setTimeout(function() { close(); }, 1000);
+setTimeout(function () {
+  setTimeout(function () {
+    throw new Error("I should never run!");
+  }, 1000);
+  close();
+}, 1000);