206561 - Memory leak on www.bhun.net

Status: CLOSED FIXED

(Core :: Web Painting, defect, P2)

Description

[bhun]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030507
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4b) Gecko/20030507

I tested the page with both Mozilla 1.3 and Mozilla 1.4b. 

When reloading the document in Mozilla 1.4b, the amount of free memory keeps
reducing. After a few reloads Mozilla crashed because there's too little memory
available. The problem does not occur in Mozilla 1.3. 

The problem is reproduceable on the Mozilla 1.4b release for the Windows
platform. (I checked)

Every reload reduces the amount of free memory with about 10-20 MB. (WOW :P)

Reproducible: Always

Steps to Reproduce:
1. Open my webpage
2. Hit reload
3. Repeat 2 until Mozilla crashes

Actual Results:  
Mozilla gets killed.

Expected Results:  
Stop claiming extra memory.

I'm not sure, but I guess it could be related to the usage of -moz-opacity in
the CSS; I really can't figure out any other reason at the moment. Furthermore I
haven't got the foggiest Idea how to use the Debug->Leak detector in the menu.

Comment 1

[Adam Hauner]

WFM 2003052004/trunk/W2K

Bhun: Could you provide TalkBack incident ID or stracktrace of crash?

Comment 2

[bhun]

The crash does not generate a Talkback incident nor does it leave it core dump.
Is there any other way to generate a stack trace? (gdb?)

Comment 3

[WD]

WFM Win2k 2003052004
The page seems to be pretty CPU-intensive for some reason, but there's no memory
leak here.

Comment 4

[Mats Palmgren (inactive)]

Before visiting http://www.bhun.net/
  PID USER     PRI  NI  SIZE  RSS SHARE STAT %CPU %MEM   TIME COMMAND
 1654 mats       9   0 48108  46M 17728 S     0.0 18.8   0:34 mozilla-bin
 1108 root      17   0 61056  11M  2132 S     5.9  4.4   0:49 X

After 1st visit:
  PID USER     PRI  NI  SIZE  RSS SHARE STAT %CPU %MEM   TIME COMMAND
 1108 root      13   0 99356  48M  2132 R     1.1 19.4   0:59 X
 1654 mats      19   0 48260  47M 17852 S     5.3 18.8   0:38 mozilla-bin

After 1 reload (simple, not SHIFT-reload):
  PID USER     PRI  NI  SIZE  RSS SHARE STAT %CPU %MEM   TIME COMMAND
 1108 root      16   0  145M  96M  2132 R     2.1 38.6   1:03 X
 1654 mats      15   0 48168  47M 17852 S     3.3 18.8   0:41 mozilla-bin

After 2 reloads:
  PID USER     PRI  NI  SIZE  RSS SHARE STAT %CPU %MEM   TIME COMMAND
 1108 root       9   0  197M 149M  2132 S    32.5 59.8   1:08 X
 1654 mats      19   0 48276  45M 17852 S    30.2 18.3   0:44 mozilla-bin

After a few more reloads the X server were using >85% mem and the
system became unusable - there was constant disk activity which I
assume was swapping. Had to reboot.

Confirming bug, 2003-05-20-22 trunk Linux. Adding keyword 'pp'.

Comment 5

[Andrew Schultz]

Attachment: testcase

<div style="height:100%;-moz-opacity:0.8;filter:Alpha(Opacity=80);"></div>

Comment 6

[Andrew Schultz]

regression between linux trunk 2003040505 and 2003040605, probably bug 113232
see bug 113232 comment 105

the testcase leaks ~2.8MB per reload.

==> views

Comment 7

[Andrew Schultz]

> The crash does not generate a Talkback incident nor does it leave it core dump.
> Is there any other way to generate a stack trace? (gdb?)

When a process uses too much RAM and the kernel kills it, there is really no way
to get a stacktrace, but what would be more useful is where Mozilla is
allocating X memory that doesn't get freed.

Comment 8

[Andrew Schultz]

it seems the rendering contexts mOffScreenCX, mOffSceenWhiteCX, mWhiteCX and
mBlackCX are the culprits here:
http://lxr.mozilla.org/mozilla/source/view/src/nsViewManager.cpp#1479

If I delete |buffers| and return before the rendering contexts are created, no X
memory is leaked.  If I delete |buffers| and return after mOffScreenCX is
created, a little bit (~8k) of X memory is leaked.  deleting |buffers| and
returning after the ____CX->FillRect(fillArea) leaks 1MB or more of X memory.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

I think this is the fix. Andrew, can you try it? Thanks!

Comment 10

[Andrew Schultz]

no X memory leaked with the patch applied.  valgrind also noticed that
|nsDrawingSurface|s were being leaked reloading the testcase.  with the patch
applied, valgrind reports no relevant leaks.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 124234 [details] [diff] [review]
fix

nsRenderingContextGTK addrefs mSurface but only releases mOffscreenSurface. 
mOffscreenSurface appears to be the "original" surface for the rendering
context, it's identical to mSurface in the original Init() codepath. Similarly
initializing it here in Init() stops us leaking the surface because the
NS_ADDREF(mSurface) is balanced by NS_IF_RELEASE(mOffscreenSurface) in the
rendering context destructor. This doesn't seem to create any new problems.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Hmm... would it be hard to just switch those members to
nsRefPtr<nsDrawingSurfaceGTK> and stop bothering with the manual addrefing and
releasing?  Should make the code more robust...'

Requesting blocking1.4 status in any case, since this is a pretty big leak and
we have several reports of it (the existing patch may be more appropriate for
the 1.4 branch than the nsRefPtr suggestion).

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'm reluctant to do such a conversion because I think the whole way drawing
surfaces are managed needs to be fixed. It's weird to have mSurface and
mOffscreenSurface usually be the same, addref mSurface and then release
mOffscreenSurface, etc.

Comment 14

[Christopher Blizzard (:blizzard)]

Comment on attachment 124234 [details] [diff] [review]
fix

r+sr=blizzard

Comment 15

[Christopher Blizzard (:blizzard)]

I, too, am worried about changing all this code over to nsCOMPtr<> since I've
been looking at this code for years and I still don't understand the ownership
model.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 124234 [details] [diff] [review]
fix

serious leak here.

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 124234 [details] [diff] [review]
fix

a=asa (on behalf of drivers) for checkin to the 1.4 branch.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Checked into trunk and branch.

Comment 19

[Roland Mainz]

Attachment: Patch for Xlib gfx (same as the GTK+ gfx patch)

Comment 20

[Roland Mainz]

Comment on attachment 124516 [details] [diff] [review]
Patch for Xlib gfx (same as the GTK+ gfx patch)

roc+moz:
Can you check the patch "in" (trunk + 1.4_branch), please ?

Comment 21

[Simon Montagu :smontagu]

Comment on attachment 124516 [details] [diff] [review]
Patch for Xlib gfx (same as the GTK+ gfx patch)

checked in on trunk.

Comment 22

[Mats Palmgren (inactive)]

Verified fixed - nightly build 2003-05-30-08 does not leak.

Comment 23

[Simon Montagu :smontagu]

Comment on attachment 124516 [details] [diff] [review]
Patch for Xlib gfx (same as the GTK+ gfx patch)

checked in on 1.4 branch

Comment 24

[bhun]

Thanks for fixing it! (I tested the new rc1 build)

Comment 25

[Jesse Ruderman]

Crashtest added as part of http://hg.mozilla.org/mozilla-central/rev/afc662d52ab1