Last Comment Bug 737195 - GC: missing barrier on JSFunction::atom
: GC: missing barrier on JSFunction::atom
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Terrence Cole [:terrence]
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-19 14:18 PDT by Terrence Cole [:terrence]
Modified: 2012-03-21 03:47 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v0 (5.94 KB, patch)
2012-03-19 14:52 PDT, Terrence Cole [:terrence]
no flags Details | Diff | Splinter Review
v1: fixed xdring of the wrapped atom (7.74 KB, patch)
2012-03-19 15:37 PDT, Terrence Cole [:terrence]
wmccloskey: review+
Details | Diff | Splinter Review

Description Terrence Cole [:terrence] 2012-03-19 14:18:30 PDT
We appear to trace through JSFunction::atom.  We only ever initialized it, so we don't need incremental barriers on it, but we do need generation barriers here.
Comment 1 Terrence Cole [:terrence] 2012-03-19 14:52:35 PDT
Created attachment 607327 [details] [diff] [review]
v0

Does this make sense?  It would be less code to just trigger the atom post barrier manually in the two places where it is needed, but this way seems cleaner.
Comment 2 [PTO to Dec5] Bill McCloskey (:billm) 2012-03-19 15:03:02 PDT
I think this misses the barrier in the js_XDRAtom case. The right way to handle that is similar to what we do with script in that function. Basically, define a local variable |atom|. In the encode case, set it to fun->atom. In the decode case, set it to NULL. Then pass &atom to js_XDRAtom. Later, in the JSXDR_DECODE section, call fun->atom.init(atom).

Related to this, I think that the fun->setScript(script) statement can be changed to initScript.
Comment 3 Terrence Cole [:terrence] 2012-03-19 15:37:44 PDT
Created attachment 607341 [details] [diff] [review]
v1: fixed xdring of the wrapped atom
Comment 4 Terrence Cole [:terrence] 2012-03-20 12:22:39 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/af799044d21e
Comment 5 Mounir Lamouri (:mounir) 2012-03-21 03:45:00 PDT
https://hg.mozilla.org/mozilla-central/rev/e367339ba6b2
Comment 6 Mounir Lamouri (:mounir) 2012-03-21 03:47:18 PDT
https://hg.mozilla.org/mozilla-central/rev/af799044d21e

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