Bug 580128 - Some more compartment fixes. r=gal

js/src/shell/js.cpp

@@ -191,6 +191,7 @@ FILE *gErrFile = NULL;
 FILE *gOutFile = NULL;
 #ifdef JS_THREADSAFE
 JSObject *gWorkers = NULL;
+js::workers::ThreadPool *gWorkerThreadPool = NULL;
 #endif
 
 static JSBool reportWarnings = JS_TRUE;
@@ -1142,8 +1143,8 @@ Quit(JSContext *cx, uintN argc, jsval *vp)
 
     gQuitting = JS_TRUE;
 #ifdef JS_THREADSAFE
-    if (gWorkers)
+    if (gWorkerThreadPool)
-        js::workers::terminateAll(cx, gWorkers);
+        js::workers::terminateAll(JS_GetRuntime(cx), gWorkerThreadPool);
 #endif
     return JS_FALSE;
 }
@@ -3809,13 +3810,8 @@ CancelExecution(JSRuntime *rt)
     if (gExitCode == 0)
         gExitCode = EXITCODE_TIMEOUT;
 #ifdef JS_THREADSAFE
-    if (gWorkers) {
+    if (gWorkerThreadPool)
-        JSContext *cx = JS_NewContext(rt, 8192);
+        js::workers::terminateAll(rt, gWorkerThreadPool);
-        if (cx) {
-            js::workers::terminateAll(cx, gWorkers);
-            JS_DestroyContextNoGC(cx);
-        }
-    }
 #endif
     JS_TriggerAllOperationCallbacks(rt);
 
@@ -5305,7 +5301,7 @@ shell(JSContext *cx, int argc, char **argv, char **envp)
     };
     ShellWorkerHooks hooks;
     if (!JS_AddNamedObjectRoot(cx, &gWorkers, "Workers") ||
-        !js::workers::init(cx, &hooks, glob, &gWorkers)) {
+        (gWorkerThreadPool = js::workers::init(cx, &hooks, glob, &gWorkers)) == NULL) {
         return 1;
     }
 #endif
@@ -5313,7 +5309,7 @@ shell(JSContext *cx, int argc, char **argv, char **envp)
     int result = ProcessArgs(cx, glob, argv, argc);
 
 #ifdef JS_THREADSAFE
-    js::workers::finish(cx, gWorkers);
+    js::workers::finish(cx, gWorkerThreadPool);
     JS_RemoveObjectRoot(cx, &gWorkers);
     if (result == 0)
         result = gExitCode;

js/src/shell/jsworkers.cpp

@@ -95,9 +95,7 @@ extern size_t gMaxStackSize;
  */
 
 namespace js {
-namespace detail {
+namespace workers {
-
-using js::workers::WorkerHooks;
 
 template <class T, class AllocPolicy>
 class Queue {
@@ -272,31 +270,44 @@ class MainQueue;
 class Event
 {
   protected:
+    virtual ~Event() { JS_ASSERT(!data); }
+
     WorkerParent *recipient;
     Worker *child;
-    JSString *data;
+    uint64 *data;
+    size_t nbytes;
 
   public:
     enum Result { fail = JS_FALSE, ok = JS_TRUE, forwardToParent };
 
-    virtual ~Event() {}
+    virtual void destroy(JSContext *cx) { 
-
+        JS_free(cx, data);
-    JSString *getData() const { return data; }
+#ifdef DEBUG
+        data = NULL;
+#endif
+        delete this;
+    }
 
     void setChildAndRecipient(Worker *aChild, WorkerParent *aRecipient) {
         child = aChild;
         recipient = aRecipient;
     }
 
+    bool deserializeData(JSContext *cx, jsval *vp) {
+        return !!JS_ReadStructuredClone(cx, data, nbytes, vp);
+    }
+
     virtual Result process(JSContext *cx) = 0;
 
     inline void trace(JSTracer *trc);
 
     template <class EventType>
     static EventType *createEvent(JSContext *cx, WorkerParent *recipient, Worker *child,
-                                  JSString *data)
+                                  jsval v)
     {
-        if (data && !JS_MakeStringImmutable(cx, data))
+        uint64 *data;
+        size_t nbytes;
+        if (!JS_WriteStructuredClone(cx, v, &data, &nbytes))
             return NULL;
 
         EventType *event = new EventType;
@@ -307,6 +318,7 @@ class Event
         event->recipient = recipient;
         event->child = child;
         event->data = data;
+        event->nbytes = nbytes;
         return event;
     }
 
@@ -323,8 +335,11 @@ class Event
             return noHandler;
 
         // Create event object.
+        jsval v;
+        if (!deserializeData(cx, &v))
+            return fail;
         JSObject *obj = JS_NewObject(cx, NULL, NULL, NULL);
-        if (!obj || !JS_DefineProperty(cx, obj, dataPropName, STRING_TO_JSVAL(data), NULL, NULL, 0))
+        if (!obj || !JS_DefineProperty(cx, obj, dataPropName, v, NULL, NULL, 0))
             return fail;
 
         // Call event handler.
@@ -345,12 +360,17 @@ class MainQueue : public EventQueue, public WorkerParent
     explicit MainQueue(ThreadPool *tp) : threadPool(tp) {}
 
     ~MainQueue() {
-        while (!queue.empty())
+        JS_ASSERT(queue.empty());
-            delete queue.pop();
     }
 
     bool init() { return initThreadSafeQueue() && initWorkerParent(); }
 
+    void destroy(JSContext *cx) {
+        while (!queue.empty())
+            queue.pop()->destroy(cx);
+        delete this;
+    }
+
     virtual JSLock *getLock() { return lock; }
     virtual ThreadPool *getThreadPool() { return threadPool; }
 
@@ -377,11 +397,15 @@ class MainQueue : public EventQueue, public WorkerParent
                 result = event->process(cx);
                 if (result == Event::forwardToParent) {
                     // FIXME - pointlessly truncates the string to 8 bits
-                    JSString *data = event->getData();
+                    jsval data;
-                    if (const char *s = data ? JS_GetStringBytesZ(cx, data) : NULL)
+                    const char *s;
+                    if (event->deserializeData(cx, &data) &&
+                        JSVAL_IS_STRING(data) &&
+                        (s = JS_GetStringBytesZ(cx, JSVAL_TO_STRING(data)))) {
                         JS_ReportError(cx, "%s", s);
-                    else
+                    } else {
                         JS_ReportOutOfMemory(cx);
+                    }
                     result = Event::fail;
                 }
                 if (result == Event::fail && continueOnError) {
@@ -392,7 +416,7 @@ class MainQueue : public EventQueue, public WorkerParent
             }
             JS_ACQUIRE_LOCK(lock);
             drop(event);
-            delete event;
+            event->destroy(cx);
             if (result != Event::ok)
                 return false;
         }
@@ -478,7 +502,7 @@ class ThreadPool
         JS_ASSERT(!mq && !wq);
         mq = new MainQueue(this);
         if (!mq || !mq->init()) {
-            delete mq;
+            mq->destroy(cx);
             mq = NULL;
             return false;
         }
@@ -486,7 +510,7 @@ class ThreadPool
         if (!wq || !wq->initThreadSafeQueue()) {
             delete wq;
             wq = NULL;
-            delete mq;
+            mq->destroy(cx);
             mq = NULL;
             return false;
         }
@@ -496,7 +520,7 @@ class ThreadPool
             threads[i] = PR_CreateThread(PR_USER_THREAD, start, wq, PR_PRIORITY_NORMAL,
                                          PR_LOCAL_THREAD, PR_JOINABLE_THREAD, 0);
             if (!threads[i]) {
-                shutdown();
+                shutdown(cx);
                 ok = false;
                 break;
             }
@@ -504,14 +528,15 @@ class ThreadPool
         return ok;
     }
 
-    void terminateAll(JSContext *cx) {
+    void terminateAll(JSRuntime *rt) {
         // See comment about JS_ATOMIC_SET in the implementation of
         // JS_TriggerOperationCallback.
         JS_ATOMIC_SET(&terminating, 1);
-        JS_TriggerAllOperationCallbacks(JS_GetRuntime(cx));
+        JS_TriggerAllOperationCallbacks(rt);
     }
 
-    void shutdown() {
+    /* This context is used only to free memory. */
+    void shutdown(JSContext *cx) {
         wq->close();
         for (int i = 0; i < threadCount; i++) {
             if (threads[i]) {
@@ -524,7 +549,7 @@ class ThreadPool
         wq = NULL;
 
         mq->disposeChildren();
-        delete mq;
+        mq->destroy(cx);
         mq = NULL;
         terminating = 0;
     }
@@ -705,7 +730,7 @@ class Worker : public WorkerParent
     void terminateSelf() {
         terminated = true;
         while (!events.empty())
-            delete events.pop();
+            events.pop()->destroy(context);
 
         // Tell the children to shut down too. An arbitrarily silly amount of
         // processing could happen before the whole tree is terminated; but
@@ -724,7 +749,7 @@ class Worker : public WorkerParent
     void dispose() {
         JS_ASSERT(!current);
         while (!events.empty())
-            delete events.pop();
+            events.pop()->destroy(context);
         if (lock) {
             JS_DESTROY_LOCK(lock);
             lock = NULL;
@@ -796,7 +821,7 @@ class Worker : public WorkerParent
                 return false;
             WorkerParent *parent = threadPool->getMainQueue();
             if (!JS_SetReservedSlot(cx, ctor, 0, PRIVATE_TO_JSVAL(parent))) {
-                threadPool->shutdown();
+                threadPool->shutdown(cx);
                 return false;
             }
             *p = parent;
@@ -826,11 +851,12 @@ class Worker : public WorkerParent
     static JSFunctionSpec jsMethods[3];
     static JSFunctionSpec jsStaticMethod[2];
 
-    static JSBool initWorkers(JSContext *cx, WorkerHooks *hooks, JSObject *global, JSObject **objp) {
+    static ThreadPool *initWorkers(JSContext *cx, WorkerHooks *hooks, JSObject *global,
+                                   JSObject **objp) {
         // Create the ThreadPool object and its JSObject wrapper.
         ThreadPool *threadPool = ThreadPool::create(cx, hooks);
         if (!threadPool)
-            return false;
+            return NULL;
 
         // Root the ThreadPool JSObject early.
         *objp = threadPool->asObject();
@@ -840,15 +866,15 @@ class Worker : public WorkerParent
                                        jsConstruct, 1,
                                        NULL, jsMethods, NULL, NULL);
         if (!proto)
-            return false;
+            return NULL;
 
         // Stash a pointer to the ThreadPool in constructor reserved slot 1.
         // It will be used later when lazily creating the MainQueue.
         JSObject *ctor = JS_GetConstructor(cx, proto);
         if (!JS_SetReservedSlot(cx, ctor, 1, PRIVATE_TO_JSVAL(threadPool)))
-            return false;
+            return NULL;
 
-        return true;
+        return threadPool;
     }
 };
 
@@ -856,11 +882,15 @@ class InitEvent : public Event
 {
   public:
     static InitEvent *create(JSContext *cx, Worker *worker, JSString *scriptName) {
-        return createEvent<InitEvent>(cx, worker, worker, scriptName);
+        return createEvent<InitEvent>(cx, worker, worker, STRING_TO_JSVAL(scriptName));
     }
 
     Result process(JSContext *cx) {
-        const char *filename = JS_GetStringBytesZ(cx, data);
+        jsval s;
+        if (!deserializeData(cx, &s))
+            return fail;
+        JS_ASSERT(JSVAL_IS_STRING(s));
+        const char *filename = JS_GetStringBytesZ(cx, JSVAL_TO_STRING(s));
         if (!filename)
             return fail;
 
@@ -878,7 +908,7 @@ class InitEvent : public Event
 class DownMessageEvent : public Event
 {
   public:
-    static DownMessageEvent *create(JSContext *cx, Worker *child, JSString *data) {
+    static DownMessageEvent *create(JSContext *cx, Worker *child, jsval data) {
         return createEvent<DownMessageEvent>(cx, child, child, data);
     }
 
@@ -890,7 +920,7 @@ class DownMessageEvent : public Event
 class UpMessageEvent : public Event
 {
   public:
-    static UpMessageEvent *create(JSContext *cx, Worker *child, JSString *data) {
+    static UpMessageEvent *create(JSContext *cx, Worker *child, jsval data) {
         return createEvent<UpMessageEvent>(cx, child->getParent(), child, data);
     }
 
@@ -925,7 +955,8 @@ class ErrorEvent : public Event
                     return NULL;
             }
         }
-        return createEvent<ErrorEvent>(cx, child->getParent(), child, data);
+        return createEvent<ErrorEvent>(cx, child->getParent(), child,
+                                       data ? STRING_TO_JSVAL(data) : JSVAL_VOID);
     }
 
     Result process(JSContext *cx) {
@@ -933,10 +964,10 @@ class ErrorEvent : public Event
     }
 };
 
-} /* namespace detail */
+} /* namespace workers */
 } /* namespace js */
 
-using namespace js::detail;
+using namespace js::workers;
 
 void
 WorkerParent::disposeChildren()
@@ -1057,7 +1088,7 @@ Worker::create(JSContext *parentcx, WorkerParent *parent, JSString *scriptName,
     if (!event)
         return NULL;
     if (!w->events.push(event) || !w->threadPool->getWorkerQueue()->post(w)) {
-        delete event;
+        event->destroy(parentcx);
         JS_ReportOutOfMemory(parentcx);
         w->dispose();
         return NULL;
@@ -1102,7 +1133,7 @@ Worker::processOneEvent()
         Event *err = ErrorEvent::create(context, this);
         if (err && !parent->post(err)) {
             JS_ReportOutOfMemory(context);
-            delete err;
+            err->destroy(context);
             err = NULL;
         }
         if (!err) {
@@ -1110,6 +1141,7 @@ Worker::processOneEvent()
         }
     }
 
+    event->destroy(context);
     JS_ClearContextThread(context);
 
     {
@@ -1121,7 +1153,6 @@ Worker::processOneEvent()
                 JS_ReportOutOfMemory(context);
         }
     }
-    delete event;
 }
 
 JSBool
@@ -1138,16 +1169,14 @@ Worker::jsPostMessageToParent(JSContext *cx, uintN argc, jsval *vp)
             return false;
     }
 
-    JSString *message = JS_ValueToString(cx, argc > 0 ? JS_ARGV(cx, vp)[0] : JSVAL_VOID);
+    jsval data = argc > 0 ? JS_ARGV(cx, vp)[0] : JSVAL_VOID;
-    if (!message)
+    Event *event = UpMessageEvent::create(cx, w, data);
-        return false;
-
-    Event *event = UpMessageEvent::create(cx, w, message);
     if (!event)
         return false;
     if (!w->parent->post(event)) {
-        delete event;
+        event->destroy(cx);
         JS_ReportOutOfMemory(cx);
+        return false;
     }
     JS_SET_RVAL(cx, vp, JSVAL_VOID);
     return true;
@@ -1166,11 +1195,8 @@ Worker::jsPostMessageToChild(JSContext *cx, uintN argc, jsval *vp)
         return false;
     }
     
-    JSString *message = JS_ValueToString(cx, argc > 0 ? JS_ARGV(cx, vp)[0] : JSVAL_VOID);
+    jsval data = argc > 0 ? JS_ARGV(cx, vp)[0] : JSVAL_VOID;
-    if (!message)
+    Event *event = DownMessageEvent::create(cx, w, data);
-        return false;
-
-    Event *event = DownMessageEvent::create(cx, w, message);
     if (!event)
         return false;
     if (!w->post(event)) {
@@ -1205,8 +1231,6 @@ Event::trace(JSTracer *trc)
         recipient->trace(trc);
     if (child)
         JS_CALL_OBJECT_TRACER(trc, child->asObject(), "worker");
-    if (data)
-        JS_CALL_STRING_TRACER(trc, data, "worker event data");
 }
 
 JSClass ThreadPool::jsClass = {
@@ -1231,29 +1255,24 @@ JSFunctionSpec Worker::jsMethods[3] = {
     JS_FS_END
 };
 
-JSBool
+ThreadPool *
 js::workers::init(JSContext *cx, WorkerHooks *hooks, JSObject *global, JSObject **rootp)
 {
     return Worker::initWorkers(cx, hooks, global, rootp);
 }
 
 void
-js::workers::terminateAll(JSContext *cx, JSObject *workersobj)
+js::workers::terminateAll(JSRuntime *rt, ThreadPool *tp)
 {
-    ThreadPool::unwrap(cx, workersobj)->terminateAll(cx);
+    tp->terminateAll(rt);
 }
 
 void
-js::workers::finish(JSContext *cx, JSObject *workersobj)
+js::workers::finish(JSContext *cx, ThreadPool *tp)
 {
-    ThreadPool *threadPool = ThreadPool::unwrap(cx, workersobj);
+    if (MainQueue *mq = tp->getMainQueue()) {
-    if (MainQueue *mq = threadPool->getMainQueue()) {
+        JS_ALWAYS_TRUE(mq->mainThreadWork(cx, true));
-#ifdef DEBUG
+        tp->shutdown(cx);
-        JSBool ok =
-#endif
-            mq->mainThreadWork(cx, true);
-        JS_ASSERT(ok);
-        threadPool->shutdown();
     }
 }
 

js/src/shell/jsworkers.h

@@ -52,6 +52,8 @@
  */
 namespace js {
     namespace workers {
+        class ThreadPool;
+
         class WorkerHooks {
         public:
             virtual JSObject *newGlobalObject(JSContext *cx) = 0;
@@ -63,17 +65,17 @@ namespace js {
 	 * Requires request. rootp must point to a GC root.
 	 *
 	 * On success, *rootp receives a pointer to an object, and init returns
-         * true. The caller must keep the object rooted and must pass it to
+         * a non-null value. The caller must keep the object rooted and must
-         * js::workers::finish later.
+         * pass it to js::workers::finish later.
 	 */
-        JSBool init(JSContext *cx, WorkerHooks *hooks, JSObject *global, JSObject **rootp);
+        ThreadPool *init(JSContext *cx, WorkerHooks *hooks, JSObject *global, JSObject **rootp);
 
         /* Asynchronously signal for all workers to terminate.
          *
          * Call this before calling finish() to shut down without waiting for
          * all messages to be proceesed.
          */
-        void terminateAll(JSContext *cx, JSObject *workersobj);
+        void terminateAll(JSRuntime *rt, ThreadPool *tp);
 
 	/*
 	 * Finish running any workers, shut down the thread pool, and free all
@@ -82,7 +84,7 @@ namespace js {
 	 *
 	 * Requires request.
 	 */
-	void finish(JSContext *cx, JSObject *workersobj);
+	void finish(JSContext *cx, ThreadPool *tp);
     }
 }