Closed Bug 994413 Opened 6 years ago Closed 6 years ago

Make the browser value tracing semantics match spidermonkey's

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: terrence, Assigned: terrence)

Details

Attachments

(1 file, 1 obsolete file)

In MarkValueInteral, we check if isMarkable and skip tracing if not markable. The browser just calls toGCThing() without any checks, leading to crashes with mis-use. This doesn't seem critical for performance if it isn't in SpiderMonkey, so we should probably just make it more useable.
Attachment #8404344 - Flags: review?(jcoppeard)
It would be nice if you could remove some or all of the existing checks. :)
Comment on attachment 8404344 [details] [diff] [review]
allow_any_value_to_be_traced-v0.diff

Review of attachment 8404344 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, I think is makes much more sense like this.
Attachment #8404344 - Flags: review?(jcoppeard) → review+
(In reply to Andrew McCreight [:mccr8] from comment #1)
> It would be nice if you could remove some or all of the existing checks. :)

Good point! I grepped for allbacks.Trace and found 2 instances to remove. Do you know of more?
Attachment #8404344 - Attachment is obsolete: true
Attachment #8405491 - Flags: review?(continuation)
Comment on attachment 8405491 [details] [diff] [review]
allow_any_value_to_be_traced-v1.diff

Review of attachment 8405491 [details] [diff] [review]:
-----------------------------------------------------------------

I can't think of any others offhand.  This should be the bulk of them.  Thanks!
Attachment #8405491 - Flags: review?(continuation) → review+
https://hg.mozilla.org/mozilla-central/rev/0312954d8974
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.