Last Comment Bug 712480 - Assert in most JSAPI functions that we're not inside a GC
: Assert in most JSAPI functions that we're not inside a GC
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
-- normal (vote)
: mozilla12
Assigned To: Bill McCloskey (:billm)
: Jason Orendorff [:jorendorff]
Depends on: 713069
  Show dependency treegraph
Reported: 2011-12-20 15:48 PST by Bill McCloskey (:billm)
Modified: 2012-01-01 21:03 PST (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (75.87 KB, patch)
2011-12-28 13:28 PST, Bill McCloskey (:billm)
jwalden+bmo: review+
Details | Diff | Splinter Review

Description User image Bill McCloskey (:billm) 2011-12-20 15:48:18 PST
Bug 711794 is a case where a finalizer is calling a JSAPI function that can GC. We should be able to assert against this.

We would have to identify a subset of the API that is safe. Things like getting reserved slots probably should be allowed during finalization. But most things should not. It probably makes sense to add asserts on every JSAPI function and then remove them as needed to get a browser running.

Brian, do you think Sixgill could help here? One problem I can foresee is that it's not obvious from Ben Turner's patch in the bug that it completely eliminates the possibility of calling XPCOM release code--in particular if the runtime is null. However, if we could annotate certain choke points like this one as being okay, then maybe it would be practical.
Comment 1 User image Bill McCloskey (:billm) 2011-12-28 13:28:18 PST
Created attachment 584629 [details] [diff] [review]

With Ben's patch applied, this is green on tryserver.
Comment 2 User image Bill McCloskey (:billm) 2011-12-29 14:19:34 PST
Comment on attachment 584629 [details] [diff] [review]

Switching to Waldo for a quicker review.
Comment 3 User image Jeff Walden [:Waldo] (remove +bmo to email) 2011-12-29 16:42:27 PST
Comment on attachment 584629 [details] [diff] [review]

Review of attachment 584629 [details] [diff] [review]:

r=me if it passes try.  And even if it does, I expect fallout for awhile from this.  But it is a Good Thing even still.
Comment 4 User image Bill McCloskey (:billm) 2012-01-01 17:21:21 PST
Yeah, we'll have to keep a close watch on this.
Comment 5 User image Phil Ringnalda (:philor) 2012-01-01 21:03:34 PST

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