909866 - Fix an exact rooting hazard in JSCompartment::wrap

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Attachment: hazard_wrap-v0.diff

This appears to simply be missing a root.

Comment 1

[Steve Fink [:sfink] [:s:]]

Comment on attachment 796155 [details] [diff] [review]
hazard_wrap-v0.diff

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

I vaguely wonder if this was a real hazard -- is the old value of str ever used within wrap()? Not that it matters.

Someday, we'll be able to update wrap() to take a handle. I remember looking briefly and deciding to do something else.

Comment 2

[Terrence Cole [:terrence]]

(In reply to Steve Fink [:sfink] from comment #1)
> 
> I vaguely wonder if this was a real hazard -- is the old value of str ever
> used within wrap()? Not that it matters.

Good question! I assumed that string wrapping could trigger GC, even if just from error reporting. I'm not sure how much it matters in this specific case.
 
> Someday, we'll be able to update wrap() to take a handle. I remember looking
> briefly and deciding to do something else.

Likewise.

Comment 3

[Terrence Cole [:terrence]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2308ce99493c

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2308ce99493c