Commits

Andrew Sutherland committed b7a73b7

no longer crashy, about to fold the de-crashy patch

Comments (0)

Files changed (2)

+# HG changeset patch
+# Parent 0d692b8bfd1fc97013b021203d922a12d4736c9c
+
+diff --git a/js/src/jsprobemgr.cpp b/js/src/jsprobemgr.cpp
+--- a/js/src/jsprobemgr.cpp
++++ b/js/src/jsprobemgr.cpp
+@@ -856,6 +856,28 @@
+     }
+ };
+ 
++class RemoveProbeEvent : public Event {
++    HandlerDescriptor *mHandleDescriptor;
++
++private:
++    RemoveProbeEvent(HandlerDescriptor *hdesc) 
++        : mHandleDescriptor(hdesc) { }
++
++public:
++    static Event *create(HandlerDescriptor *hdesc) {
++        return new RemoveProbeEvent(hdesc);
++    }
++
++    virtual Result process(JSContext *cx) {
++        ProbeManager *mgr = static_cast<ProbeManager *>(JS_GetContextPrivate(cx));
++        JS_ASSERT(mgr != NULL);
++
++        mgr->dispatchProbeRemoval(cx, mHandleDescriptor);
++        mHandleDescriptor = 0;
++        return ok;
++    }
++};
++
+ class EventQueue : public ThreadSafeQueue<Event *> {
+ public:
+     ~EventQueue() { JS_ASSERT(queue.empty()); }
+@@ -1447,8 +1469,8 @@
+             /* this should not happen because we try and compile it on add */
+             if (!desc.handlerFun)
+                 continue;
+-            JSObject *handlerFunObj = JS_GetFunctionObject(desc.handlerFun);
+-            JS_AddNamedObjectRoot(cx, &handlerFunObj, "probe");
++            desc.handlerFunObj = JS_GetFunctionObject(desc.handlerFun);
++            JS_AddNamedObjectRoot(cx, &desc.handlerFunObj, "probe");
+         }
+         JS_CallFunction(cx, globalObj, desc.handlerFun, argc, argv, &rval);
+     }
+@@ -1564,35 +1586,58 @@
+ bool ProbeManager::removeProbeHandler(HandlerCookie cookie)
+ {
+     HandlerDescriptor *hdesc;
++    {
++        /* steal handler queue lock */
++        AutoLock hold(eq->getLock());
++        JSAutoRequest req(adminCx);
+ 
+-    /* steal handler queue lock */
+-    AutoLock hold(eq->getLock());
+-    JSAutoRequest req(adminCx);
++        /* look up handler by cookie, remove if it exists */
++        if (HandlerMap::Ptr p = m_handlerMap->lookup(cookie)) {
++            hdesc = p->value;
++            m_handlerMap->remove(p);
++        } else {
++            PR_LOG(gProbeManagerLog, PR_LOG_ERROR, ("ProbeManager: Tried to remove handler with bad cookie: %d\n", cookie));
++            return false;
++        }
+ 
+-    /* look up handler by cookie, remove if it exists */
+-    if (HandlerMap::Ptr p = m_handlerMap->lookup(cookie)) {
+-        hdesc = p->value;
+-        m_handlerMap->remove(p);
+-    } else {
+-        PR_LOG(gProbeManagerLog, PR_LOG_ERROR, ("ProbeManager: Tried to remove handler with bad cookie: %d\n", cookie));
++        removeHandler(hdesc);
++        /* TODO: post a cleanup message to the processing thread so it can remove
++           the root:
++        JS_RemoveObjectRoot(adminCx, &hdesc->handlerFunObj);
++        */
++
++        /* null out the allocations made on this thread */
++        JS_free(adminCx, hdesc->argnames);
++        JS_free(adminCx, const_cast<jschar *>(hdesc->handlerDef.body));
++        JS_free(adminCx, const_cast<char *>(hdesc->handlerDef.filename));
++        hdesc->argnames = 0;
++        hdesc->handlerDef.body = 0;
++        hdesc->handlerDef.filename = 0;
++    }
++
++    /* create an event for cleanup that must happen on the processing thread */
++    Event *event = RemoveProbeEvent::create(hdesc);
++    if (!eq->post(event)) {
++        delete event;
++        JS_ReportOutOfMemory(adminCx);
+         return false;
+     }
+ 
+-    removeHandler(hdesc);
+-    /* TODO: post a cleanup message to the processing thread so it can remove
+-       the root:
+-    JSObject *handlerFunObj = JS_GetFunctionObject(hdesc->handlerFun);
+-    JS_RemoveObjectRoot(adminCx, &handlerFunObj);
+-    */
+-
+-    JS_free(adminCx, hdesc->argnames);
+-    JS_free(adminCx, const_cast<jschar *>(hdesc->handlerDef.body));
+-    JS_free(adminCx, const_cast<char *>(hdesc->handlerDef.filename));
+-
+-    /* return status */
+     return true;
+ }
+ 
++bool ProbeManager::dispatchProbeRemoval(JSContext *cx,
++                                        HandlerDescriptor *hdesc) {
++    JS_RemoveObjectRoot(cx, &hdesc->handlerFunObj);
++    hdesc->handlerFun = 0;
++
++    delete hdesc;
++
++    return true;
++}
++
++/* XXX this method's removal semantics are not safe, but no one calls it
++   either */
+ bool ProbeManager::clearProbeHandlers(ProbePoint probe)
+ {
+     /* steal handler queue lock */
+diff --git a/js/src/jsprobemgr.h b/js/src/jsprobemgr.h
+--- a/js/src/jsprobemgr.h
++++ b/js/src/jsprobemgr.h
+@@ -64,6 +64,8 @@
+ struct HandlerDescriptor {
+     /* created on the processing thread the first time it is used */
+     JSFunction *handlerFun;
++    /* function object for the handler that can serve as a GC root */
++    JSObject   *handlerFunObj;
+     /* the script info to be used to compile handlerFun */
+     ScriptDescriptor handlerDef;
+     uintN argc;
+@@ -197,6 +199,7 @@
+     bool runScriptOnce(const ScriptDescriptor &fdesc, ScriptCookie *scriptCookie);
+     bool addProbeHandler(ProbePoint probe, ScriptDescriptor &fdesc, ScriptDescriptor &specInfo, HandlerCookie* handlerCookie);
+     bool removeProbeHandler(HandlerCookie cookie);
++    bool dispatchProbeRemoval(JSContext *cx, HandlerDescriptor *hdesc);
+     bool clearProbeHandlers(ProbePoint probe);
+ 
+     /* TODO: this should always be inlined into the Probes::whateverThing function
 js-run-probes
 provide-all-compartment-ptrs
 jetpack-scriptcontext-naked-jscontext
+fix-wrong-gc-rooting
 jetpack-scriptcontext-problem-snafu