Bug 890841 - Intermittent dom/workers/test/test_closeOnGC.html | application crashed [@ libsystem_c.dylib + 0x19bd9]. r=sicking, a=akeybl.
authorBen Turner <bent.mozilla@gmail.com>
Fri, 18 Oct 2013 16:37:25 -0700
changeset 154252 8321acab48097c4d6e420478e9a025a15eb785b7
parent 154251 eeea6cc5c54ad0cebfe156be56e03a4aeefceb33
child 154253 b1a16b230cbe2e421d7207a19803e514e92ebe78
push id2944
push userbturner@mozilla.com
push dateFri, 18 Oct 2013 23:39:09 +0000
treeherdermozilla-beta@8321acab4809 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssicking, akeybl
bugs890841
milestone25.0
Bug 890841 - Intermittent dom/workers/test/test_closeOnGC.html | application crashed [@ libsystem_c.dylib + 0x19bd9]. r=sicking, a=akeybl.
dom/workers/RuntimeService.cpp
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -1802,18 +1802,27 @@ RuntimeService::ResumeWorkersForWindow(n
                                        nsPIDOMWindow* aWindow)
 {
   AssertIsOnMainThread();
 
   nsAutoTArray<WorkerPrivate*, 100> workers;
   GetWorkersForWindow(aWindow, workers);
 
   if (!workers.IsEmpty()) {
+    nsCOMPtr<nsIScriptGlobalObject> sgo = do_QueryInterface(aWindow);
+    MOZ_ASSERT(sgo);
+
+    nsIScriptContext* scx = sgo->GetContext();
+
+    AutoPushJSContext cx(scx ?
+                         scx->GetNativeContext() :
+                         nsContentUtils::GetSafeJSContext());
+
     for (uint32_t index = 0; index < workers.Length(); index++) {
-      if (!workers[index]->SynchronizeAndResume(aCx)) {
+      if (!workers[index]->SynchronizeAndResume(cx, aWindow, scx)) {
         NS_WARNING("Failed to cancel worker!");
       }
     }
   }
 }
 
 void
 RuntimeService::NoteIdleThread(nsIThread* aThread)
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -45,17 +45,16 @@
 #include "nsError.h"
 #include "nsDOMJSUtils.h"
 #include "nsGUIEvent.h"
 #include "nsJSEnvironment.h"
 #include "nsJSUtils.h"
 #include "nsNetUtil.h"
 #include "nsProxyRelease.h"
 #include "nsSandboxFlags.h"
-#include "nsThreadUtils.h"
 #include "xpcpublic.h"
 
 #ifdef ANDROID
 #include <android/log.h>
 #endif
 
 #include "Events.h"
 #include "Exceptions.h"
@@ -1437,43 +1436,16 @@ public:
   bool
   WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
   {
     aWorkerPrivate->GarbageCollectInternal(aCx, mShrinking, mCollectChildren);
     return true;
   }
 };
 
-class SynchronizeAndResumeRunnable : public nsRunnable
-{
-protected:
-  WorkerPrivate* mWorkerPrivate;
-  nsCOMPtr<nsIScriptContext> mCx;
-
-public:
-  SynchronizeAndResumeRunnable(WorkerPrivate* aWorkerPrivate,
-                               nsIScriptContext* aCx)
-  : mWorkerPrivate(aWorkerPrivate), mCx(aCx)
-  {
-    NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
-  }
-
-  NS_IMETHOD Run()
-  {
-    NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
-
-    AutoPushJSContext cx(mCx ? mCx->GetNativeContext() :
-                         nsContentUtils::GetSafeJSContext());
-    JSAutoRequest ar(cx);
-    mWorkerPrivate->Resume(cx);
-
-    return NS_OK;
-  }
-};
-
 class WorkerJSRuntimeStats : public JS::RuntimeStats
 {
   const nsACString& mRtPath;
 
 public:
   WorkerJSRuntimeStats(const nsACString& aRtPath)
   : JS::RuntimeStats(JsWorkerMallocSizeOf), mRtPath(aRtPath)
   { }
@@ -1745,16 +1717,68 @@ WorkerRunnable::NotifyScriptExecutedIfNe
   if (mTarget == ParentThread && !mWorkerPrivate->GetParent()) {
     AssertIsOnMainThread();
     if (mWorkerPrivate->GetScriptNotify()) {
       mWorkerPrivate->GetScriptNotify()->ScriptExecuted();
     }
   }
 }
 
+template <class Derived>
+class WorkerPrivateParent<Derived>::SynchronizeAndResumeRunnable
+  : public nsRunnable
+{
+  friend class WorkerPrivateParent<Derived>;
+  friend class nsRevocableEventPtr<SynchronizeAndResumeRunnable>;
+
+  WorkerPrivate* mWorkerPrivate;
+  nsCOMPtr<nsPIDOMWindow> mWindow;
+  nsCOMPtr<nsIScriptContext> mScriptContext;
+
+  SynchronizeAndResumeRunnable(WorkerPrivate* aWorkerPrivate,
+                               nsPIDOMWindow* aWindow,
+                               nsIScriptContext* aScriptContext)
+  : mWorkerPrivate(aWorkerPrivate), mWindow(aWindow),
+    mScriptContext(aScriptContext)
+  {
+    AssertIsOnMainThread();
+    MOZ_ASSERT(aWorkerPrivate);
+    MOZ_ASSERT(aWindow);
+    MOZ_ASSERT(!aWorkerPrivate->GetParent());
+  }
+
+  NS_IMETHOD
+  Run()
+  {
+    AssertIsOnMainThread();
+
+    if (mWorkerPrivate) {
+      AutoPushJSContext cx(mScriptContext ?
+                           mScriptContext->GetNativeContext() :
+                           nsContentUtils::GetSafeJSContext());
+
+      mWorkerPrivate->Resume(cx);
+    }
+
+    return NS_OK;
+  }
+
+  void
+  Revoke()
+  {
+    AssertIsOnMainThread();
+    MOZ_ASSERT(mWorkerPrivate);
+    MOZ_ASSERT(mWindow);
+
+    mWorkerPrivate = nullptr;
+    mWindow = nullptr;
+    mScriptContext = nullptr;
+  }
+};
+
 struct WorkerPrivate::TimeoutInfo
 {
   TimeoutInfo()
   : mTimeoutVal(JS::UndefinedValue()), mLineNumber(0), mId(0), mIsInterval(false),
     mCanceled(false)
   {
     MOZ_COUNT_CTOR(mozilla::dom::workers::WorkerPrivate::TimeoutInfo);
   }
@@ -2017,16 +2041,20 @@ WorkerPrivateParent<Derived>::NotifyPriv
       self->SetThread(currentThread);
     }
 #endif
     // Worker never got a chance to run, go ahead and delete it.
     self->ScheduleDeletion(true);
     return true;
   }
 
+  // Only top-level workers should have a synchronize runnable.
+  MOZ_ASSERT_IF(mSynchronizeRunnable.get(), !GetParent());
+  mSynchronizeRunnable.Revoke();
+
   NS_ASSERTION(aStatus != Terminating || mQueuedRunnables.IsEmpty(),
                "Shouldn't have anything queued!");
 
   // Anything queued will be discarded.
   mQueuedRunnables.Clear();
 
   nsRefPtr<NotifyRunnable> runnable =
     new NotifyRunnable(ParentAsWorkerPrivate(), aStatus);
@@ -2067,16 +2095,20 @@ WorkerPrivateParent<Derived>::Resume(JSC
   {
     MutexAutoLock lock(mMutex);
 
     if (mParentStatus >= Terminating) {
       return;
     }
   }
 
+  // Only top-level workers should have a synchronize runnable.
+  MOZ_ASSERT_IF(mSynchronizeRunnable.get(), !GetParent());
+  mSynchronizeRunnable.Revoke();
+
   // Execute queued runnables before waking up the worker, otherwise the worker
   // could post new messages before we run those that have been queued.
   if (!mQueuedRunnables.IsEmpty()) {
     AssertIsOnMainThread();
 
     nsTArray<nsRefPtr<WorkerRunnable> > runnables;
     mQueuedRunnables.SwapElements(runnables);
 
@@ -2088,30 +2120,40 @@ WorkerPrivateParent<Derived>::Resume(JSC
 
   nsRefPtr<ResumeRunnable> runnable =
     new ResumeRunnable(ParentAsWorkerPrivate());
   runnable->Dispatch(aCx);
 }
 
 template <class Derived>
 bool
-WorkerPrivateParent<Derived>::SynchronizeAndResume(nsIScriptContext* aCx)
+WorkerPrivateParent<Derived>::SynchronizeAndResume(
+                                               JSContext* aCx,
+                                               nsPIDOMWindow* aWindow,
+                                               nsIScriptContext* aScriptContext)
 {
-  AssertIsOnParentThread();
+  AssertIsOnMainThread();
   NS_ASSERTION(mParentSuspended, "Not yet suspended!");
 
   // NB: There may be pending unqueued messages.  If we resume here we will
   // execute those messages out of order.  Instead we post an event to the
   // end of the event queue, allowing all of the outstanding messages to be
   // queued up in order on the worker.  Then and only then we execute all of
   // the messages.
 
   nsRefPtr<SynchronizeAndResumeRunnable> runnable =
-    new SynchronizeAndResumeRunnable(ParentAsWorkerPrivate(), aCx);
-  return NS_SUCCEEDED(NS_DispatchToCurrentThread(runnable));
+    new SynchronizeAndResumeRunnable(ParentAsWorkerPrivate(), aWindow,
+                                     aScriptContext);
+  if (NS_FAILED(NS_DispatchToCurrentThread(runnable))) {
+    JS_ReportError(aCx, "Failed to dispatch to current thread!");
+    return false;
+  }
+
+  mSynchronizeRunnable = runnable;
+  return true;
 }
 
 template <class Derived>
 void
 WorkerPrivateParent<Derived>::_trace(JSTracer* aTrc)
 {
   // This should only happen on the parent thread but we can't assert that
   // because it can also happen on the cycle collector thread when this is a
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -1,51 +1,49 @@
 /* -*- Mode: c++; c-basic-offset: 2; indent-tabs-mode: nil; tab-width: 40 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef mozilla_dom_workers_workerprivate_h__
 #define mozilla_dom_workers_workerprivate_h__
 
-#include "mozilla/Attributes.h"
 #include "Workers.h"
 
 #include "nsIContentSecurityPolicy.h"
 #include "nsIRunnable.h"
 #include "nsIThread.h"
 #include "nsIThreadInternal.h"
 #include "nsPIDOMWindow.h"
 
 #include "jsapi.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/CondVar.h"
-#include "mozilla/Mutex.h"
 #include "mozilla/TimeStamp.h"
 #include "nsAutoPtr.h"
 #include "nsCOMPtr.h"
 #include "nsEventQueue.h"
 #include "nsStringGlue.h"
 #include "nsTArray.h"
+#include "nsThreadUtils.h"
 #include "nsTPriorityQueue.h"
 #include "StructuredCloneTags.h"
 
 #include "EventTarget.h"
 #include "Queue.h"
 #include "WorkerFeature.h"
 
 class JSAutoStructuredCloneBuffer;
 class nsIChannel;
 class nsIDocument;
 class nsIMemoryMultiReporter;
 class nsIPrincipal;
 class nsIScriptContext;
+class nsITimer;
 class nsIURI;
-class nsPIDOMWindow;
-class nsITimer;
 class nsIXPCScriptNotify;
 
 namespace JS {
 class RuntimeStats;
 }
 
 BEGIN_WORKERS_NAMESPACE
 
@@ -233,16 +231,18 @@ public:
     MOZ_ASSERT(mMutex);
     mMutex->AssertCurrentThreadOwns();
   }
 };
 
 template <class Derived>
 class WorkerPrivateParent : public EventTarget
 {
+  class SynchronizeAndResumeRunnable;
+
 public:
   struct LocationInfo
   {
     nsCString mHref;
     nsCString mProtocol;
     nsCString mHost;
     nsCString mHostname;
     nsCString mPort;
@@ -271,16 +271,17 @@ private:
   nsCOMPtr<nsIURI> mBaseURI;
   nsCOMPtr<nsIURI> mScriptURI;
   nsCOMPtr<nsIPrincipal> mPrincipal;
   nsCOMPtr<nsIChannel> mChannel;
   nsCOMPtr<nsIContentSecurityPolicy> mCSP;
 
   // Only used for top level workers.
   nsTArray<nsRefPtr<WorkerRunnable> > mQueuedRunnables;
+  nsRevocableEventPtr<SynchronizeAndResumeRunnable> mSynchronizeRunnable;
 
   // Only for ChromeWorkers without window and only touched on the main thread.
   nsTArray<nsCString> mHostObjectURIs;
 
   // Protected by mMutex.
   JSSettings mJSSettings;
 
   uint64_t mBusyCount;
@@ -352,17 +353,18 @@ public:
 
   bool
   Suspend(JSContext* aCx);
 
   void
   Resume(JSContext* aCx);
 
   bool
-  SynchronizeAndResume(nsIScriptContext* aCx);
+  SynchronizeAndResume(JSContext* aCx, nsPIDOMWindow* aWindow,
+                       nsIScriptContext* aScriptContext);
 
   virtual void
   _trace(JSTracer* aTrc) MOZ_OVERRIDE;
 
   virtual void
   _finalize(JSFreeOp* aFop) MOZ_OVERRIDE;
 
   void