There are a lot of unrooted XPCCallContext warnings in xpconnect. This class is only allocated on the stack, but is not straightforward because by design it doesn't always have initialised GC thing pointers, and doesn't always have a JSContext*. The JSContext used is decided by some hairy looking logic on initialisation. My approach was is to add CustomAutoRooter, an AutoRooter for use outside the JS engine. This has a virtual trace() method that derived classes can implement to do whatever root marking is necessary. In XPCCallContext I added a derived AutoXPCCallContextRooter and added this as a Maybe<...> member so that it can be constructed only if necessary. Terrence, can you let me know if you think this is a good way to proceed? You may have other plans for rooting non-trivial stack objects outside the JS engine.

This approach looks fine to me, but I have a rather high tolerance for hacks. Bobby, how close are we to having a single JSContext in the browser? How close are we if a few of us dig in and help you get the work done? Is there some other way - even a hackish one - for us to reliably get a hold of any random context that we could use in the meantime?

(In reply to Terrence Cole [:terrence] from comment #2) > Bobby, how close are we to having a single JSContext in the browser? How > close are we if a few of us dig in and help you get the work done? > Is there > some other way - even a hackish one - for us to reliably get a hold of any > random context that we could use in the meantime? Sure. If you just need to root and you don't really care what the cx is, you can just use the safe JSContext, or even better the stack-top cx. That is to say, |AutoJSContext cx;| should solve your issue.

(In reply to Bobby Holley (:bholley) from comment #3) > (In reply to Terrence Cole [:terrence] from comment #2) > > > Bobby, how close are we to having a single JSContext in the browser? How > > close are we if a few of us dig in and help you get the work done? Er, meant to respond to this - I don't think you want to block on the single-cxing work. There are a lot of really nasty deps, and mostly ones that require someone like me or bz to fix. I'm working through them, but it's kind of at the bottom of my stack.

Comment on attachment 730221 [details] [diff] [review] Add CustomAutoRooter class Review of attachment 730221 [details] [diff] [review]: ----------------------------------------------------------------- This looks pretty awesome. Before this lands, however, I'd like to see a few uses of it in engine to replace our overly-specific AutoRooters, as we discussed in IRC. ::: js/src/jsapi.h @@ +178,5 @@ > + static void markObjectRoot(JSTracer *trc, JSObject **thingp, const char *name); > + static void markScriptRoot(JSTracer *trc, JSScript **thingp, const char *name); > + static void markStringRoot(JSTracer *trc, JSString **thingp, const char *name); > + static void markIdRoot(JSTracer *trc, jsid *thingp, const char *name); > + static void markValueRoot(JSTracer *trc, JS::Value *thingp, const char *name); I would leave off the Root postfix: users outside the engine will be confused about when they should call these and when they should call JS_CallFooTracer. In fact, we should probably call them callFooTracer to match the other API and reduce confusion between "tracing" and "marking".

(In reply to Bobby Holley (:bholley) from comment #3) > That is to say, |AutoJSContext cx;| should solve your issue.

Here's an updated patch to the JS engine with comments addressed. I've removed changes to xpconnect from this bug.

Comment on attachment 732324 [details] [diff] [review] Proposed changes Review of attachment 732324 [details] [diff] [review]: ----------------------------------------------------------------- Very nice! r=me once you've made the two changes below. ::: js/src/gc/RootMarking.cpp @@ -573,5 @@ > - case REGEXPSTATICS: { > - RegExpStatics::AutoRooter *rooter = static_cast<RegExpStatics::AutoRooter *>(this); > - rooter->trace(trc); > - return; > - } \o/ @@ +634,5 @@ > + MarkValueRoot(trc, thingp, name); > +} > + > +void > +PropDesc::AutoRooter::trace(JSTracer *trc) Could you move each of the Foo::AutoRooter::trace methods here inline into the definition of the AutoRooter? I see a huge part of the value of the CustomAutoRooter coming from the fact that it allows us to move the tracing we are causing closer to the data that is being traced. ::: js/src/vm/Shape.h @@ +855,5 @@ > { > public: > Inner(JSContext *cx, uint8_t attrs, > PropertyOp *pgetter_, StrictPropertyOp *psetter_) > + : CustomAutoRooter(cx), attrs(attrs), Might as well fix the spacing here while making this change.

(In reply to Terrence Cole [:terrence] from comment #8) Thanks! Comments addressed, except I couldn't move StackShape::AutoRooter::trace() inline because that created a cyclic header file dependency when including gc/Marking.h

Backed out for crashes. https://hg.mozilla.org/integration/mozilla-inbound/rev/32125e0a9954 https://tbpl.mozilla.org/php/getParsedLog.php?id=21386508&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=21386585&tree=Mozilla-Inbound etc...

Attempting to reland. https://hg.mozilla.org/integration/mozilla-inbound/rev/d77817dcc9ef