Bug 676376 - 'prevent multi-threaded JSRuntime access in new web worker memory reporters'. r=luke+sicking.

dom/workers/RuntimeService.cpp

@@ -291,16 +291,16 @@ CreateJSContextForWorker(WorkerPrivate* aWorkerPrivate)
 
 class WorkerMemoryReporter : public nsIMemoryMultiReporter
 {
-  JSRuntime* mRuntime;
+  WorkerPrivate* mWorkerPrivate;
   nsCString mPathPrefix;
 
 public:
   NS_DECL_ISUPPORTS
 
-  WorkerMemoryReporter(WorkerPrivate* aWorkerPrivate, JSRuntime* aRuntime)
+  WorkerMemoryReporter(WorkerPrivate* aWorkerPrivate)
-  : mRuntime(aRuntime)
+  : mWorkerPrivate(aWorkerPrivate)
   {
-    NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!");
+    aWorkerPrivate->AssertIsOnWorkerThread();
 
     nsCString escapedDomain(aWorkerPrivate->Domain());
     escapedDomain.ReplaceChar('/', '\\');
@@ -331,10 +331,8 @@ public:
   {
     AssertIsOnMainThread();
 
-    JS_TriggerAllOperationCallbacks(mRuntime);
-
     IterateData data;
-    if (!CollectCompartmentStatsForRuntime(mRuntime, &data)) {
+    if (!mWorkerPrivate->BlockAndCollectRuntimeStats(&data)) {
       return NS_ERROR_FAILURE;
     }
 
@@ -372,16 +370,17 @@ public:
       return NS_ERROR_FAILURE;
     }
 
-    JSRuntime* rt = JS_GetRuntime(cx);
-
     nsRefPtr<WorkerMemoryReporter> reporter =
-      new WorkerMemoryReporter(workerPrivate, rt);
+      new WorkerMemoryReporter(workerPrivate);
     if (NS_FAILED(NS_RegisterMemoryMultiReporter(reporter))) {
       NS_WARNING("Failed to register memory reporter!");
       reporter = nsnull;
     }
 
+    {
+      JSAutoRequest ar(cx);
       workerPrivate->DoRunLoop(cx);
+    }
 
     if (reporter) {
       if (NS_FAILED(NS_UnregisterMemoryMultiReporter(reporter))) {
@@ -390,6 +389,8 @@ public:
       reporter = nsnull;
     }
 
+    JSRuntime* rt = JS_GetRuntime(cx);
+
     // XXX Bug 666963 - CTypes can create another JSContext for use with
     // closures, and then it holds that context in a reserved slot on the CType
     // prototype object. We have to destroy that context before we can destroy

dom/workers/WorkerPrivate.cpp

@@ -64,6 +64,7 @@
 #include "nsJSUtils.h"
 #include "nsNetUtil.h"
 #include "nsThreadUtils.h"
+#include "xpcpublic.h"
 
 #include "Events.h"
 #include "Exceptions.h"
@@ -83,6 +84,7 @@ using mozilla::MutexAutoLock;
 using mozilla::TimeDuration;
 using mozilla::TimeStamp;
 using mozilla::dom::workers::exceptions::ThrowDOMExceptionForCode;
+using mozilla::xpconnect::memory::IterateData;
 
 USING_WORKERS_NAMESPACE
 
@@ -1070,6 +1072,60 @@ public:
 };
 #endif
 
+class CollectRuntimeStatsRunnable : public WorkerControlRunnable
+{
+  typedef mozilla::Mutex Mutex;
+  typedef mozilla::CondVar CondVar;
+
+  Mutex* mMutex;
+  CondVar* mCondVar;
+  volatile bool* mDoneFlag;
+  IterateData* mData;
+  volatile bool* mSucceeded;
+
+public:
+  CollectRuntimeStatsRunnable(WorkerPrivate* aWorkerPrivate, Mutex* aMutex,
+                              CondVar* aCondVar, volatile bool* aDoneFlag,
+                              IterateData* aData, volatile bool* aSucceeded)
+  : WorkerControlRunnable(aWorkerPrivate, WorkerThread, UnchangedBusyCount),
+    mMutex(aMutex), mCondVar(aCondVar), mDoneFlag(aDoneFlag), mData(aData),
+    mSucceeded(aSucceeded)
+  { }
+
+  bool
+  PreDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
+  {
+    AssertIsOnMainThread();
+    return true;
+  }
+
+  void
+  PostDispatch(JSContext* aCx, WorkerPrivate* aWorkerPrivate,
+               bool aDispatchResult)
+  {
+    AssertIsOnMainThread();
+  }
+
+  bool
+  WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate)
+  {
+    JSAutoSuspendRequest asr(aCx);
+
+    *mSucceeded = CollectCompartmentStatsForRuntime(JS_GetRuntime(aCx), mData);
+
+    {
+      MutexAutoLock lock(*mMutex);
+
+      NS_ASSERTION(!*mDoneFlag, "Should be false!");
+
+      *mDoneFlag = true;
+      mCondVar->Notify();
+    }
+
+    return true;
+  }
+};
+
 } /* anonymous namespace */
 
 #ifdef DEBUG
@@ -2042,8 +2098,6 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
       {
         MutexAutoUnlock unlock(mMutex);
 
-        JSAutoRequest ar(aCx);
-
 #ifdef EXTRA_GC
         // Find GC bugs...
         JS_GC(aCx);
@@ -2056,8 +2110,6 @@ WorkerPrivate::DoRunLoop(JSContext* aCx)
       currentStatus = mStatus;
     }
 
-    JSAutoRequest ar(aCx);
-
 #ifdef EXTRA_GC
     // Find GC bugs...
     JS_GC(aCx);
@@ -2097,8 +2149,6 @@ WorkerPrivate::OperationCallback(JSContext* aCx)
 {
   AssertIsOnWorkerThread();
 
-  JS_YieldRequest(aCx);
-
   bool mayContinue = true;
 
   for (;;) {
@@ -2133,7 +2183,6 @@ WorkerPrivate::OperationCallback(JSContext* aCx)
         break;
       }
 
-      JSAutoSuspendRequest asr(aCx);
       mCondVar.Wait(PR_MillisecondsToInterval(RemainingRunTimeMS()));
     }
   }
@@ -2184,6 +2233,36 @@ WorkerPrivate::ScheduleDeletion(bool aWasPending)
   }
 }
 
+bool
+WorkerPrivate::BlockAndCollectRuntimeStats(IterateData* aData)
+{
+  AssertIsOnMainThread();
+  mMutex.AssertNotCurrentThreadOwns();
+  NS_ASSERTION(aData, "Null data!");
+
+  mozilla::Mutex mutex("BlockAndCollectRuntimeStats mutex");
+  mozilla::CondVar condvar(mutex, "BlockAndCollectRuntimeStats condvar");
+  volatile bool doneFlag = false;
+  volatile bool succeeded = false;
+
+  nsRefPtr<CollectRuntimeStatsRunnable> runnable =
+    new CollectRuntimeStatsRunnable(this, &mutex, &condvar, &doneFlag, aData,
+                                    &succeeded);
+  if (!runnable->Dispatch(nsnull)) {
+    NS_WARNING("Failed to dispatch runnable!");
+    return false;
+  }
+
+  {
+    MutexAutoLock lock(mutex);
+    while (!doneFlag) {
+      condvar.Wait();
+    }
+  }
+
+  return succeeded;
+}
+
 bool
 WorkerPrivate::Dispatch(WorkerRunnable* aEvent, EventQueue* aQueue)
 {
@@ -2488,7 +2567,6 @@ WorkerPrivate::RunSyncLoop(JSContext* aCx, PRUint32 aSyncLoopKey)
       MutexAutoLock lock(mMutex);
 
       while (!mControlQueue.Pop(event) && !syncQueue->mQueue.Pop(event)) {
-        JSAutoSuspendRequest asr(aCx);
         mCondVar.Wait();
       }
     }

dom/workers/WorkerPrivate.h

@@ -68,6 +68,16 @@ class nsIURI;
 class nsPIDOMWindow;
 class nsITimer;
 
+namespace mozilla {
+namespace xpconnect {
+namespace memory {
+
+struct IterateData;
+
+} // namespace memory
+} // namespace xpconnect
+} // namespace mozilla
+
 BEGIN_WORKERS_NAMESPACE
 
 class WorkerPrivate;
@@ -640,6 +650,9 @@ public:
   void
   ScheduleDeletion(bool aWasPending);
 
+  bool
+  BlockAndCollectRuntimeStats(mozilla::xpconnect::memory::IterateData* aData);
+
 #ifdef JS_GC_ZEAL
   void
   UpdateGCZealInternal(JSContext* aCx, PRUint8 aGCZeal);

js/src/xpconnect/src/xpcjsruntime.cpp

@@ -1323,8 +1323,8 @@ CompartmentCallback(JSContext *cx, void *vdata, JSCompartment *compartment)
     // Append a new CompartmentStats to the vector.
     IterateData *data = static_cast<IterateData *>(vdata);
     CompartmentStats compartmentStats(cx, compartment);
-    data->compartmentStatsVector.infallibleAppend(compartmentStats);
+    CompartmentStats *curr =
-    CompartmentStats *curr = data->compartmentStatsVector.end() - 1;
+        data->compartmentStatsVector.AppendElement(compartmentStats);
     data->currCompartmentStats = curr;
 
     // Get the compartment-level numbers.
@@ -1610,8 +1610,7 @@ CollectCompartmentStatsForRuntime(JSRuntime *rt, IterateData *data)
     {
         JSAutoRequest ar(cx);
 
-        if (!data->compartmentStatsVector.reserve(rt->compartments.length()))
+        data->compartmentStatsVector.SetCapacity(rt->compartments.length());
-            return false;
 
         data->gcHeapChunkCleanUnused =
             PRInt64(JS_GetGCParameter(rt, JSGC_UNUSED_CHUNKS)) *
@@ -1635,17 +1634,19 @@ CollectCompartmentStatsForRuntime(JSRuntime *rt, IterateData *data)
                                    data->gcHeapChunkCleanUnused;
     data->gcHeapArenaUnused = 0;
 
-    for(CompartmentStats *stats = data->compartmentStatsVector.begin();
+    for(PRUint32 index = 0;
-        stats != data->compartmentStatsVector.end();
+        index < data->compartmentStatsVector.Length();
-        ++stats)
+        index++)
     {
-        data->gcHeapChunkDirtyUnused -=
+        CompartmentStats &stats = data->compartmentStatsVector[index];
-            stats->gcHeapArenaHeaders + stats->gcHeapArenaPadding +
-            stats->gcHeapArenaUnused +
-            stats->gcHeapObjects + stats->gcHeapStrings +
-            stats->gcHeapShapes + stats->gcHeapXml;
 
-        data->gcHeapArenaUnused += stats->gcHeapArenaUnused;
+        data->gcHeapChunkDirtyUnused -=
+            stats.gcHeapArenaHeaders + stats.gcHeapArenaPadding +
+            stats.gcHeapArenaUnused +
+            stats.gcHeapObjects + stats.gcHeapStrings +
+            stats.gcHeapShapes + stats.gcHeapXml;
+        
+        data->gcHeapArenaUnused += stats.gcHeapArenaUnused;
     }
 
     size_t numDirtyChunks = (data->gcHeapChunkTotal -
@@ -1808,11 +1809,12 @@ ReportJSRuntimeStats(const IterateData &data, const nsACString &pathPrefix,
                      nsIMemoryMultiReporterCallback *callback,
                      nsISupports *closure)
 {
-    for(const CompartmentStats *stats = data.compartmentStatsVector.begin();
+    for(PRUint32 index = 0;
-        stats != data.compartmentStatsVector.end();
+        index < data.compartmentStatsVector.Length();
-        ++stats)
+        index++)
     {
-        ReportCompartmentStats(*stats, pathPrefix, callback, closure);
+        ReportCompartmentStats(data.compartmentStatsVector[index], pathPrefix,
+                               callback, closure);
     }
 
     ReportMemoryBytes(pathPrefix + NS_LITERAL_CSTRING("stack"),

js/src/xpconnect/src/xpcpublic.h

@@ -49,6 +49,7 @@
 #include "nsIPrincipal.h"
 #include "nsWrapperCache.h"
 #include "nsStringGlue.h"
+#include "nsTArray.h"
 
 class nsIPrincipal;
 
@@ -242,7 +243,7 @@ struct IterateData
     PRInt64 gcHeapChunkAdmin;
     PRInt64 gcHeapUnusedPercentage;
 
-    js::Vector<CompartmentStats, 0, js::SystemAllocPolicy> compartmentStatsVector;
+    nsTArray<CompartmentStats> compartmentStatsVector;
     CompartmentStats *currCompartmentStats;
 };