Closed
Bug 665044
Opened 12 years ago
Closed 12 years ago
Don't garbage collect while we hold the nsCycleCollectorRunner mutex
Categories
(Core :: XPCOM, defect)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla7
People
(Reporter: mccr8, Assigned: mccr8)
References
Details
Attachments
(1 file)
1.22 KB,
patch
|
bent.mozilla
:
review+
|
Details | Diff | Splinter Review |
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.
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
Assignee | ||
Comment 2•12 years ago
|
||
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
Assignee | ||
Comment 3•12 years ago
|
||
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.
Assignee | ||
Comment 4•12 years ago
|
||
Assignee | ||
Comment 5•12 years ago
|
||
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
Assignee | ||
Updated•12 years ago
|
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+
Assignee | ||
Comment 7•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/771d948c8779
Comment 8•12 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/771d948c8779
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
You need to log in
before you can comment on or make changes to this bug.
Description
•