From IRC today: <dbaron> So it seems like one of the problem areas that WAY_TOO_MUCH_GC doesn't catch is when various things within the JS engine do things that call out to a JS API user (e.g., something in XPConnect) who is permitted to allocate JS objects, but usually doesn't. Should there be any effort to document which callbacks are allowed to allocate and add js_GC calls #ifdef WAY_TOO_MUCH_GC? Brendan and I talked about this, and doing it for the JS engine in general is not something brendan really wants to do. What he's willing to do is to expose an API that will do exactly what we want here (not poke (to attempt to not hit perf any more than we already do), but clear newborns and then gc). Then we could call this API at least from the XPCWrappedNative jsops (from the macros those use). Preferably we should also look at other JSClass definitions we have and decide whether those ops should call this API. Places that should be checked if we do that: A few places in XPConnect. A few places in nsDOMClassInfo (global scope polluter, constructor, etc) XBL David, what do you think?

(In reply to comment #0) > <dbaron> So it seems like one of the problem areas that WAY_TOO_MUCH_GC doesn't > catch is when various things within the JS engine do things that call out to > a > JS API user (e.g., something in XPConnect) who is permitted to allocate JS > objects, but usually doesn't. Should there be any effort to document which > callbacks are allowed to allocate and add js_GC calls #ifdef WAY_TOO_MUCH_GC? Just to be crystal clear: every callback is allowed to call any JS API entry point, in general. Counterexamples welcome. I'm probably forgetting something, but the full API is what we expose, and Murphy's Law says every hook will call the least convenient API at the worst time. IOW we haven't designed any implicit restrictions into the system that are in need of documentation. /be

It is nice to be able to define/undefine WAY_TOO_MUCH_GC and rebuild only in js/src/, though.

It's already used in nsJSEnvironment.cpp...

We could catch many of them just by making the JS_PropertyStub and friends force a GC #if WAY_TOO_MUCH_GC, I suspect. There are a handful of other cases (debugger callouts, etc.) that aren't stubbed, but I suspect we could audit our way to victory there without undue pain. (Counterexample (?): finalizers can't allocate, and I suspect that GC-notification callbacks can't either, looking quickly at the test in js_NewGCThing.)

Should we be setting rt->gcPoke in JS_ClearNewbortRoots and when setting newborns in js_NewGCThing?

(In reply to comment #4) > We could catch many of them just by making the JS_PropertyStub and friends > force a GC #if WAY_TOO_MUCH_GC, I suspect. JS_PropertyStub is not called at all for stubbed getters, setters, and addProperty calls.. I continue to think we want to avoid diminishing returns here. Fixing bug 313437 would be a better use of time at some point :-P. /be

(In reply to comment #5) > Should we be setting rt->gcPoke in JS_ClearNewbortRoots and when setting > newborns in js_NewGCThing? #ifdef WAY_TOO_MUCH_GC, maybe in the first case. In the second, I'm not sure. That might cross the line into unbearable slowness (WAY_WAY_TOO_MUCH_GC?). /be

*** Bug 333119 has been marked as a duplicate of this bug. ***

(In reply to comment #6) > JS_PropertyStub is not called at all for stubbed getters, setters, and > addProperty calls.. > > I continue to think we want to avoid diminishing returns here. Fixing bug > 313437 would be a better use of time at some point :-P. IMO it worths to spend some time for an option to force GC whenever JS-defined getter, setter or object convereter could be called. It helps to prevent GC bugs that scripts can exploit to subvert jsval type in few lines of code. Resolving bug 313437 is nice but it would not prevent bugs, say, in jsval->JSString conversion code as they would be bugs in the engine, not in application callbacks.

We could macroize the JSClass hook calls, for sure. That would be a JS engine patch, not an XPConnect patch. Or were people thinking only of patching XPConnect somehow? /be

WAY_TOO_MUCH_GC doesn't exist any more.