Closed Bug 665044 Opened 11 years ago Closed 11 years ago

Don't garbage collect while we hold the nsCycleCollectorRunner mutex

Categories

(Core :: XPCOM, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla7

People

(Reporter: mccr8, Assigned: mccr8)

References

Details

Attachments

(1 file)

The cycle collector calls the JS garbage collector while holding a mutex.  Bug 664506 separates the JS GC call a bit from the rest of the CC, so bent would like to look into moving the JS GC call outside of the mutex.
Depends on: 664506
To be clear, it's the nsCycleCollectorRunner's mutex. The cycle collector doesn't have a mutex.
Summary: Don't garbage collect while we hold the cycle collector mutex → Don't garbage collect while we hold the nsCycleCollectorRunner mutex
I can work on this, but I'd like to discuss with somebody what exactly the runner mutex is supposed to do.
Assignee: nobody → continuation
Looking over the code, the only thing it looks like the main thread and the CC thread will touch in GCIfNeeded is the GC status bits, but the only time the CC thread read that info is when it is in BeginCollection, and that will only happen when the main thread is sitting between mRequest.Notify() and mReply.Wait(), where there's no code running, so it looks okay to me.

One thing I'm not sure about is that lifting the GC above |if (!mCollector->PrepareForCollection(&whiteNodes))|, but that means the GC will happen before obs->NotifyObservers(nsnull, "cycle-collector-begin", nsnull), instead of after.  Is that okay?  I don't know what it does.
Try run was good on this. (I redid the Android failure and it came out okay, despite not showing up on the chart.)
http://tbpl.mozilla.org/?tree=Try&rev=440d90d06dc1
Attachment #540907 - Flags: review?(bent.mozilla)
Comment on attachment 540907 [details] [diff] [review]
lift the CC GC outside of the runner mutex

Review of attachment 540907 [details] [diff] [review]:
-----------------------------------------------------------------
Attachment #540907 - Flags: review?(bent.mozilla) → review+
http://hg.mozilla.org/mozilla-central/rev/771d948c8779
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
You need to log in before you can comment on or make changes to this bug.