Closed
Bug 703474
Opened 9 years ago
Closed 9 years ago
Setting gczeal to 4 and back to 0 causes everything to leak
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: jruderman, Assigned: billm)
References
Details
(Keywords: memory-leak, testcase)
Attachments
(3 files)
To reproduce in a debug browser build: 1. Install https://www.squarefree.com/extensions/domFuzzLite3.xpi 2. XPCOM_MEM_LEAK_LOG=2 ./firefox v.html --> trace-refcnt reports that nsGlobalWindow and more leak To reproduce in the shell using valgrind: 1. Build a spidermonkey shell: a. Create a new directory b. js/src/configure --enable-debug --disable-optimize --enable-valgrind c. make 2. valgrind --leak-check=full --dsymutil=yes ./js -e "gczeal(4); gczeal(0);" --> valgrind reports a significant leak rooted in js::gc::StartVerifyBarriers
Reporter | ||
Comment 1•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Assignee: general → wmccloskey
Reporter | ||
Comment 2•9 years ago
|
||
It worries me that turning on GC debugging makes the GC stop working ;)
Assignee | ||
Comment 3•9 years ago
|
||
I remember fixing the shell problem a while ago, although I can't find the bug. It no longer reproduces. The problem in the browser case was that, even though we set the gczeal back to 0, we were still calling Start/EndVerifyBarriers repeatedly. I had thought that the verifier would stop at the next GC. However, if a GC happens during a verification, it starts a new one after the GC is done. The current patch adds a few places where we EndVerifyBarriers if the zeal is 0. I considered just calling EndVerifyBarriers from SetGCZeal, but I'd rather have this happen from places where we already know that GC is safe. Also, I made sure not to start a verification if we do a CC_FORCED GC, since it relies on the mark bits being correct afterwards.
Attachment #608552 -
Flags: review?(igor)
Comment 4•9 years ago
|
||
Comment on attachment 608552 [details] [diff] [review] fix Review of attachment 608552 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsgc.cpp @@ +3693,5 @@ > } > + ~AutoVerifyBarriers() { > + if (inVerify && reason != gcreason::CC_FORCED && > + cx->runtime->gcZeal() == ZealVerifierValue) > + { There is no reason to store inVerify and reason as they stay the same. Instead store the result of inVerify && reason != gcreason::CC_FORCED in a flag. Also comment here why it is necessary to protect StartVerifyBarriers(cx) with the extra checks.
Attachment #608552 -
Flags: review?(igor) → review+
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca74029146c2
Target Milestone: --- → mozilla14
Comment 6•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ca74029146c2
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•