Assert in most JSAPI functions that we're not inside a GC

RESOLVED FIXED in mozilla12

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: billm, Assigned: billm)

Tracking

unspecified
mozilla12
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Updated

5 years ago
Depends on: 713069
(Assignee)

Comment 1

5 years ago
Created attachment 584629 [details] [diff] [review]
patch

With Ben's patch applied, this is green on tryserver.
Attachment #584629 - Flags: review?(luke)
(Assignee)

Comment 2

5 years ago
Comment on attachment 584629 [details] [diff] [review]
patch

Switching to Waldo for a quicker review.
Attachment #584629 - Flags: review?(luke) → review?(jwalden+bmo)
Comment on attachment 584629 [details] [diff] [review]
patch

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.
Attachment #584629 - Flags: review?(jwalden+bmo) → review+
(Assignee)

Comment 4

5 years ago
Yeah, we'll have to keep a close watch on this.

https://hg.mozilla.org/integration/mozilla-inbound/rev/edffd43801d9
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/edffd43801d9
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.