Created attachment 687813 [details] [diff] [review] patch With bug 815906 landed, decoder is seeing assertions where we are still trying to mark things from the compilation thread. What I think is happening is: 1. During a global incremental GC, a new compartment is created between IGC slices. That compartment does not have needsBarrier() set. 2. Still before the next slice starts, we try to compile code in that compartment, and do it off thread because barriers are not set on the compartment. 3. Off thread compilation writes an atom somewhere (don't know where, don't have a full stack), which triggers a write barrier on the atoms compartment since it has needsBarrier() set. I don't know the GC well enough to determine whether this is actually possible, i.e. whether compartments created during an IGC don't interrupt the GC or set needsBarrier() on the new compartment. From cursory examination of the code this seems to be the case though, at least in the JS shell. The attached patch is a spotfix to avoid compiling code in compartments that don't need barriers but can point to the atoms compartment, which does. It would be nice though if we maintained an invariant that this wasn't actually possible, as it seems weird given that compartments can freely maintain pointers into the atoms compartment.
Created attachment 687829 [details] [diff] [review] better patch
Comment on attachment 687829 [details] [diff] [review] better patch Review of attachment 687829 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/Ion.cpp @@ +1182,5 @@ > // performed off thread during an incremental GC, as doing so may trip > // incremental read barriers. > if (js_IonOptions.parallelCompilation && > OffThreadCompilationAvailable(cx) && > + cx->runtime->gcIncrementalState == gc::NO_INCREMENTAL) I just remembered that we have IsIncrementalGCInProgress(rt), which is slightly better than this. Please use that.
Attachment #687829 - Flags: review?(wmccloskey) → review+
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
This fix didn't have the desired effect, as IsIncrementalGCInProgress returns false if the barrier verifier is running, which also enables barriers on compartments and can cause marking to occur off thread. Using the original cx->runtime->gcIncrementalState == gc::NO_INCREMENTAL test instead should fix this.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Revert to original patch (r=billm over IRC): https://hg.mozilla.org/integration/mozilla-inbound/rev/1e3e9febdc2e
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago → 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.