Last Comment Bug 685007 - initialize mark bits in Chunk::init
: initialize mark bits in Chunk::init
Status: RESOLVED FIXED
[qa-]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla7
Assigned To: Rafael Ávila de Espíndola (:espindola) (not reading bugmail)
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 673017
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-06 15:25 PDT by Andrew McCreight [:mccr8]
Modified: 2011-09-09 15:52 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
fixed


Attachments
initialize mark bits in Chunk::init (699 bytes, patch)
2011-09-06 15:25 PDT, Andrew McCreight [:mccr8]
continuation: review+
christian: approval‑mozilla‑aurora+
christian: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Andrew McCreight [:mccr8] 2011-09-06 15:25:11 PDT
Created attachment 558637 [details] [diff] [review]
initialize mark bits in Chunk::init

In Bug 673017, Rafael found that uninitialized mark bits were being read by xpc_UnmarkGrayObject.  He has a patch for it.  This bug is mostly just so I can nominate that patch for Aurora and Beta.

The general problem is that xpc_UnmarkGrayObject and the cycle collector proper rely on mark bits being valid, but the bits are uninitialized from when a chunk is allocated until garbage collection is run.  One specific case of this, chunks allocated from startup being examined by the CC before the GC has a chance to run, is explicitly handled by checking if the GC has run when the CC starts up, and running the GC if not.  This does not handle everything.

Aside from the case Rafael found, one troubling possibility is that a chunk can be allocated at some point after the first GC, then have objects allocated in it, then the CC runs and examines those objects, all before the GC runs.  It will be reading random uninitialized mark bits, which I think this has the potential for causing things to be freed when they shouldn't be.

Bill found that this didn't have any noticeable performance impact.

I've attached Rafael's patch as committed to m-c: http://hg.mozilla.org/mozilla-central/rev/f092ce58bc20
Comment 1 christian 2011-09-06 16:53:19 PDT
Comment on attachment 558637 [details] [diff] [review]
initialize mark bits in Chunk::init

Approved for mozilla-beta and mozilla-aurora. If there are any issues we'll back it out. Please land by early tomorrow.
Comment 2 Andrew McCreight [:mccr8] 2011-09-06 17:21:47 PDT
I'll land this on beta tonight.
Comment 3 Andrew McCreight [:mccr8] 2011-09-06 18:13:39 PDT
http://hg.mozilla.org/releases/mozilla-beta/rev/7b67433ffa2f
Comment 4 Andrew McCreight [:mccr8] 2011-09-07 08:50:52 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/abcf25487703

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