Last Comment Bug 518675 - JSAutoTempValueRooter(...) is bad mojo
: JSAutoTempValueRooter(...) is bad mojo
Status: RESOLVED FIXED
[sg:critical?] (possible gc race cond...
: verified1.9.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: P1 critical (vote)
: ---
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-09-24 14:31 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2009-11-09 18:36 PST (History)
5 users (show)
sayrer: blocking1.9.2+
dveditz: wanted1.9.0.x-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1-fixed
.4+
.4-fixed


Attachments
Patch (2.11 KB, patch)
2009-09-24 14:44 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
dvander: review+
dveditz: approval1.9.1.4+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-24 14:31:55 PDT
Creates a root, immediately unroots, value expected to be protected, isn't.  Yikes.
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-24 14:44:23 PDT
Created attachment 402673 [details] [diff] [review]
Patch
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-24 15:28:17 PDT
http://hg.mozilla.org/tracemonkey/rev/33825a77eba8
Comment 3 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-24 15:32:34 PDT
Comment on attachment 402673 [details] [diff] [review]
Patch

This is minimal enough that it could easily be added to 1.9.1.4, if sufficient time remains, without any meaningful worries.  I leave it up to approvers to consider whether it's worthwhile -- it'd be hard to get the failure precisely so for it to matter, but I think it is worthwhile to do it now rather than give people extra time to play with this.

Since this is a C++-only failure 1.9.0 is not affected; I presume a 1.9.2 merge by sayrer will pick this up in due course.
Comment 4 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-24 15:55:31 PDT
Could use a merge to m-c, leaving to the traditional merger so as not to cross the streams...
Comment 5 Daniel Veditz [:dveditz] 2009-09-25 10:33:19 PDT
If this is a potential security problem we should hide the bug. We've treated this kind of problem as potentially [sg:critical?] in the past so we should hide the bug until it's fixed.
Comment 6 Daniel Veditz [:dveditz] 2009-09-28 14:53:17 PDT
Comment on attachment 402673 [details] [diff] [review]
Patch

Approved for 1.9.1.4, a=dveditz for release-drivers

trivial fix, better safe than sorry.
Comment 7 Jeff Walden [:Waldo] (remove +bmo to email) 2009-09-28 19:00:52 PDT
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/eedb768cfbb8
Comment 8 Robert Sayre 2009-09-28 19:04:12 PDT
looks like at least one person has hit this:

http://crash-stats.mozilla.com/report/index/a7412eac-60f6-4c0f-8706-ec6282090922
Comment 9 Al Billings [:abillings] 2009-09-29 10:47:07 PDT
Verified for 1.9.1 in source.

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