Closed
Bug 812422
Opened 12 years ago
Closed 11 years ago
Valgrind on tbpl detects leak - 96 bytes are definitely lost (direct) with NS_NewComputedDOMStyle and nsGlobalWindow::GetComputedStyleHelper on the stack, due to leaking entire layout module
Categories
(Core :: General, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: gkw, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: memory-leak, regression, valgrind)
Attachments
(1 file)
(deleted),
text/plain
|
Details |
Valgrind detects a leak of 96 bytes (direct) with NS_NewComputedDOMStyle and nsGlobalWindow::GetComputedStyleHelper on the stack, see attached snippet which comes from:
https://tbpl.mozilla.org/php/getParsedLog.php?id=17064538&tree=Firefox&full=1
m-c changeset rev is a761bfc192b5.
It didn't happen in the previous Valgrind run here: https://tbpl.mozilla.org/?noignore=1&jobname=valgrind&rev=dd68409d7810
So I'm guessing the regression range is:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dd68409d7810&tochange=a761bfc192b5
Reporter | ||
Comment 1•12 years ago
|
||
Guessing Core: DOM, please feel free to change components if necessary.
Comment 2•12 years ago
|
||
Bug 115199 is in the range, and has "style" in the name, but it doesn't seem to directly touch a computed style file.
I looked at the stack and there's some element stuff near the call site:
8256 nsCOMPtr<dom::Element> element = do_QueryInterface(aElt);
8257 NS_ENSURE_TRUE(element, NS_ERROR_FAILURE);
8258 nsRefPtr<nsComputedDOMStyle> compStyle =
8259 NS_NewComputedDOMStyle(element, aPseudoElt, presShell,
8260 aDefaultStylesOnly ? nsComputedDOMStyle::eDefaultOnly :
8261 nsComputedDOMStyle::eAll);
8262
8263 *aReturn = compStyle.forget().get();
8264
So maybe bug 811449 or bug 807075 are more likely.
Component: DOM → Style System (CSS)
Comment 3•12 years ago
|
||
Isn't that just because we're leaking nsLayoutStatics and don't deallocate the sCachedComputedDOMStyle as a result? I doubt the nsComputedDOMStyle is the real issue.
Comment 4•12 years ago
|
||
The leaking argument objects and XPCWrappedNativeScopes seem more serious to me.
Comment 5•12 years ago
|
||
Yeah, Peter is right. We leaked the layout module as a whole here....
Component: Style System (CSS) → General
Summary: Valgrind on tbpl detects leak - 96 bytes are definitely lost (direct) with NS_NewComputedDOMStyle and nsGlobalWindow::GetComputedStyleHelper on the stack → Valgrind on tbpl detects leak - 96 bytes are definitely lost (direct) with NS_NewComputedDOMStyle and nsGlobalWindow::GetComputedStyleHelper on the stack, due to leaking entire layout module
Reporter | ||
Comment 6•12 years ago
|
||
Nominating for tracking since leaking the whole layout module sounds bad.
status-firefox19:
--- → affected
tracking-firefox19:
--- → ?
Comment 7•12 years ago
|
||
I'm not sure it is that bad. We always hold this alive until shutdown anyways.
Comment 8•12 years ago
|
||
(In reply to Andrew McCreight [:mccr8] from comment #7)
> I'm not sure it is that bad. We always hold this alive until shutdown
> anyways.
Given this, not tracking for FF19. Please re-nominate if the landscape changes.
Comment 9•11 years ago
|
||
This is no longer occurring in Valgrind-on-TBPL runs.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•