Last Comment Bug 477598 - Make XPCWrappedJSClass::CallMethod use public rooting APIs
: Make XPCWrappedJSClass::CallMethod use public rooting APIs
: sec-other
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Other Branch
: All All
-- normal (vote)
: ---
Assigned To: Blake Kaplan (:mrbkap)
: Andrew Overholt [:overholt]
Depends on:
  Show dependency treegraph
Reported: 2009-02-09 06:56 PST by Jason Orendorff [:jorendorff]
Modified: 2012-05-04 08:03 PDT (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Description User image Jason Orendorff [:jorendorff] 2009-02-09 06:56:01 PST
There's this comment in XPCWrappedJSClass::CallMethod:

  // We use js_AllocStack, js_Invoke, and js_FreeStack so that the gcthings
  // we use as args will be rooted by the engine as we do conversions and
  // prepare to do the function call. This adds a fair amount of complexity,
  // but is a good optimization compared to calling JS_AddRoot for each item.

I don't think this is correct anymore.  In jsgc.cpp it seems like js_TraceContext only traces those parts of the stack that are pointed-to by JSStackFrames.  The right thing here, I think, would be a JSAutoTempValueRooter.

Incidentally, I think making that change would also fix bug 476643.
Comment 1 User image Blake Kaplan (:mrbkap) 2009-02-09 14:23:16 PST
I think we should do this, but there's no GC hazard here. js_AllocStack adds an entry to cx->stackHeaders, which is scanned on trace.
Comment 2 User image Blake Kaplan (:mrbkap) 2009-02-09 14:23:42 PST
By "on trace," I mean "on GC."
Comment 3 User image Blake Kaplan (:mrbkap) 2012-05-04 08:03:55 PDT
This happened over a series of patches a while back.

Note You need to log in before you can comment on or make changes to this bug.