793602 - Valgrind on tbpl detects leak at malloc (104 bytes in 1 blocks are definitely lost) with nsGlobalWindow::GetComputedStyle on the stack

Status: RESOLVED WORKSFORME

(Core :: DOM: CSS Object Model, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: Valgrind stack

Valgrind detects a leak of 104 bytes at malloc with nsGlobalWindow::GetComputedStyle on the stack, see attached snippet which comes from:

https://tbpl.mozilla.org/php/getParsedLog.php?id=15471615&tree=Firefox

Guessing Core: DOM, please change component if necessary.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is quite odd.  How does the leak-detection get done here?  Any idea which test is triggering it?

We do have a static one-object cache for this stuff, but we delete it at shutdown...  We do cast it around a bit in the process; is this confusing valgrind?

Comment 2

[Julian Seward [:jseward]]

(In reply to Boris Zbarsky (:bz) from comment #1)
> We do have a static one-object cache for this stuff, but we delete it at
> shutdown...  We do cast it around a bit in the process; is this confusing

I doubt it, since V simply analyses machine code, and doesn't look at source.

Is it possible that Fx is doing fork() and the child is exiting quickly?
That can give strange leak check results.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I don't know.

But my real question is whether the machine code produced from this:

  delete reinterpret_cast<char*>(sCachedComputedDOMStyle);

where we have:

  static nsComputedDOMStyle *sCachedComputedDOMStyle;

and earlier on we did:

  sCachedComputedDOMStyle = aComputedStyle;
  sCachedComputedDOMStyle->~nsComputedDOMStyle();

will generate machine code that actually calls "free" on the same pointer as just "delete aComputedStyle" would.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The suppression for this was commented out a few days ago to try and get stacks with line numbers, but it did not show up again.

Resolving WFM, removing suppression, and will file a new bug with stacks that have line numbers if it shows up again.