Closed Bug 812422 Opened 8 years ago Closed 7 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)

x86_64
Linux
defect
Not set
major

Tracking

()

RESOLVED WORKSFORME
Tracking Status
firefox19 - affected

People

(Reporter: gkw, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: memory-leak, regression, valgrind)

Attachments

(1 file)

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
Guessing Core: DOM, please feel free to change components if necessary.
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)
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.
The leaking argument objects and XPCWrappedNativeScopes seem more serious to me.
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
Nominating for tracking since leaking the whole layout module sounds bad.
I'm not sure it is that bad. We always hold this alive until shutdown anyways.
(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.
This is no longer occurring in Valgrind-on-TBPL runs.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.