148879 - Crash when browsing large local directory M100 branch [@ nsImageWin::DrawComposited24 ] [@ nsImageWin::CreateImageWithAlphaBits ]

Status: VERIFIED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[John S. Musarra]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0.0) Gecko/20020602
BuildID:    2002060206

I decided to test out Moz on a really big dir, so I pointed it at my images
directory, which contains over 1500 files. (no, it's *not* pr0n) Moz loaded the
dir okay, but upon scrolling, the sidebar graphics went kinda flooey - the bar
bg turned white, and the up and down arrows and the 'part of the scrollbar
that's the thing that you grab with the mouse and drag up and down" (sorry, I
don't know what it's called) turned to featureless bluish (modern-colored)
squares. Even with the sidebar corruption, I could still scroll and view the
file listings, but the files had lost their associated icons. I decided to close
the tab, and did so by clicking the X, and presto, crash.

Reproducible: Always
Steps to Reproduce:
1. type file://c:/ in teh URL bar
2. browse to mypics directory
3. close tab

Actual Results:  graphics corruption and crash

Expected Results:  Directory should display properly and browser should not crash

Comment 1

[Sander]

Confirmed once navigating to file:///C:/WINDOWS/system/ containing 1369 files.
Massive repaint problems in the chrome, as well as in the system tray and parts
of other windows; crash on closing the tab.
After closing a lot of apps and retrying it doesn't seem to happen anymore
however...
win98 RC3 - 2002052306

Comment 2

[Sander]

Almost immediately after confirming I was able to reproduce again; TB6965806E

Comment 3

[Bernard Alleysson]

I can confirm this on Win XP Pro (both mozilla RC3 and trunk 2002060322)
In fact there is no crash on Win NT because it is much more robust on GDI
resource leak:

Steps:
1. Go to file:///C:/WINDOWS/system32/ 
2. Open NT Task manager, View | Select columns... and choose GDI Objects, then OK
3. Note GDI Objects count as you reload or scroll the file listing in Mozilla

Results:
GDI Objects increase. When it gets to #10000 then you icons are not displayed
anymore in the file listing and in the chrome. You have big display problems.

Expected:
Do not leak GDI Objects. No display problem.

Comment 4

[John S. Musarra]

This is the Talkback ID from the initial report:
TB6965044Y

Comment 5

[greer]

Product ID  Gecko1.0
Reassigning to Karnaze (on the assumption that Marc is not available). Chris,
pls reassign if I have erred.

With the few crash incidents that we have in the three days of data on the M100
branch, this one makes the topcrash charts. However, if it does not increase in
frequency we might minus it. For now -> topcrash.

Here's the stack info from the incident in the previous comment:

Build ID 2002060209
Trigger Time 2002-06-03 15:46:39
Platform Win32
Operating System Windows 98 4.10 build 67766446
Module GKGFXWIN.DLL
URL visited file://C:/
User Comments reproducing previous crash
Trigger Reason Access violation
Source File Name d:\builds\seamonkey\mozilla\gfx\src\windows\nsImageWin.cpp
Trigger Line No. 647

Stack Trace
nsImageWin::DrawComposited24
[d:\builds\seamonkey\mozilla\gfx\src\windows\nsImageWin.cpp, line 647]
nsImageWin::DrawComposited
[d:\builds\seamonkey\mozilla\gfx\src\windows\nsImageWin.cpp, line 677]
nsImageWin::Draw [d:\builds\seamonkey\mozilla\gfx\src\windows\nsImageWin.cpp,
line 534]
nsRenderingContextImpl::DrawImage
[d:\builds\seamonkey\mozilla\gfx\src\nsRenderingContextImpl.cpp, line 923]
nsImageBoxFrame::PaintImage
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsImageBoxFrame.cpp, line 533]
nsImageBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsImageBoxFrame.cpp, line 481]
nsBoxFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1699]
nsBoxFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1836]
nsBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1652]
nsBoxFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1699]
nsBoxFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1836]
nsBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1652]
nsBoxFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1699]
nsBoxFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1836]
nsBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1652]
nsBoxFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1699]
nsBoxFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1836]
nsBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1652]
nsBoxFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1699]
nsBoxFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1836]
nsBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1652]
nsBoxFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1699]
nsBoxFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1836]
nsBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1652]
nsBoxFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1699]
nsBoxFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1836]
nsBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1652]
nsBoxFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1699]
nsBoxFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1836]
nsBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1652]
nsBoxFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1699]
nsBoxFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1836]
nsBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1652]
nsBoxFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1699]
nsBoxFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1836]
nsBoxFrame::Paint
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp, line 1652]
nsContainerFrame::PaintChild
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line 256]
nsContainerFrame::PaintChildren
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line 197]
nsContainerFrame::Paint
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp, line 178]
PresShell::Paint
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp, line 5804]
nsView::Paint [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 280]
nsViewManager::RenderDisplayListElement
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp, line 1192]
nsViewManager::RenderViews
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp, line 1141]
nsViewManager::Refresh [d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp,
line 732]
nsViewManager::DispatchEvent
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp, line 1732]
HandleEvent [d:\builds\seamonkey\mozilla\view\src\nsView.cpp, line 83]
nsWindow::DispatchEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 971]
nsWindow::DispatchWindowEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 993]
nsWindow::OnPaint [d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp,
line 4622]
nsWindow::ProcessMessage
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 3501]
nsWindow::WindowProc
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp, line 1233]
KERNEL32.DLL + 0x363b (0xbff7363b)
KERNEL32.DLL + 0x24407 (0xbff94407) 

Comment 6

[karnaze (gone)]

The stack is top heavy with gfx and xul. Reassigning to gfx.

Comment 7

[Kevin McCluskey (gone)]

-> dcone

Comment 8

[greer]

Following Bernard's steps in comment #3, when my GDI # reached 9,999 I clicked
on a bookmark to Bugzilla and got this crash on the Trunk:

Stack Signature  nsImageWin::CreateImageWithAlphaBits 47e9eb36
Email Address [e-mail]
Product ID MozillaTrunk
Build ID 2002060308
Trigger Time 2002-06-04 11:40:42
Platform Win32
Operating System Windows NT 5.0 build 2195
Module gkgfxwin.dll
URL visited fiel://c:/WINNT/system32/
User Comments Using steps in comment #3 of bug 148879
Trigger Reason Access violation
Source File Name nsImageWin.cpp
Trigger Line No. 362
Stack Trace
nsImageWin::CreateImageWithAlphaBits [nsImageWin.cpp, line 362]
nsImageWin::CreateDDB [nsImageWin.cpp, line 391]
nsImageWin::Draw [nsImageWin.cpp, line 460]
nsRenderingContextImpl::DrawImage [nsRenderingContextImpl.cpp, line 923]
nsImageBoxFrame::PaintImage [nsImageBoxFrame.cpp, line 532]
nsImageBoxFrame::Paint [nsImageBoxFrame.cpp, line 480]
PresShell::Paint [nsPresShell.cpp, line 5822]
nsView::Paint [nsView.cpp, line 280]
nsViewManager::RenderDisplayListElement [nsViewManager.cpp, line 1192]
nsViewManager::RenderViews [nsViewManager.cpp, line 1141]
nsViewManager::Refresh [nsViewManager.cpp, line 732]
nsViewManager::DispatchEvent [nsViewManager.cpp, line 1732]
HandleEvent [nsView.cpp, line 83]
nsWindow::DispatchEvent [nsWindow.cpp, line 973]
nsWindow::DispatchWindowEvent [nsWindow.cpp, line 995]
nsWindow::OnPaint [nsWindow.cpp, line 4633]
nsWindow::ProcessMessage [nsWindow.cpp, line 3507]
nsWindow::WindowProc [nsWindow.cpp, line 1235]
USER32.DLL + 0x3eb0 (0x77e13eb0)
USER32.DLL + 0x591b (0x77e1591b)
USER32.DLL + 0x595d (0x77e1595d)
ntdll.dll + 0x1fb83 (0x77f9fb83)
USER32.DLL + 0x92da (0x77e192da)
nsAppShellService::Run [nsAppShellService.cpp, line 451]
main1 [nsAppRunner.cpp, line 1472]
main [nsAppRunner.cpp, line 1808]
WinMain [nsAppRunner.cpp, line 1826]
WinMainCRTStartup()
KERNEL32.DLL + 0x7903 (0x77e87903) 

Comment 9

[greer]

The crash that I mention above is reproducible. topcrash -> topcrash+
cc'ing dp, cathleen, and garrett per Shiva.

Comment 10

[Shiva Thirumazhusai]

Bugzillla is acting weird. I just added myself to the cc list and bugzilla
changed bunch of stuff.  I have corrected them back. 

 Here are the new things I have done.
  nominating for RTM. 
  Added ADT impact.

Comment 11

[ly nguyen]

okay so here's what i have so far.

greer's most recent call stack says that the death happens in nsImageWin.cpp,
line 362 but lxr will not display the correct line it's dying on. instead it'll
show the end of a struct definition. the correct line number is 408 using lxr's
line count. the following loop dies on first iteration (y = 0).

401     for (int y = 0; y < mBHead->biHeight; y++) {
402       unsigned char *imageWithAlphaRow = imageWithAlphaBits + y * 
                                             mBHead->biWidth * 4;
403       unsigned char *imageRow = mImageBits + y * mRowBytes;
404       unsigned char *alphaRow = mAlphaBits + y * mARowBytes;
405 
406       for (int x = 0; x < mBHead->biWidth;
407           x++, imageWithAlphaRow += 4, imageRow += 3, alphaRow++) {
408         FAST_DIVIDE_BY_255(imageWithAlphaRow[0], imageRow[0] * *alphaRow);

using the debugger, imageWithAlphaRow is 0 which is because imageWithAlphaBits
is 0. seems like it won't have a null crash once y > 0 but not sure. weird that
imageWithAlphaBits (local var) is 0. i'm gonna investigate some more...

Comment 12

[Bernard Alleysson]

I think that the crash is related to some GDI Object resource leak.
Why GDI Objects count always increases ? Is it normal ? I don't think so.
If this is a leak then the crash occurs far away from its root cause on an
attempt to grab some sort of GDI resource (HICON, HBITMAP, HDC, HFONT, ...)
If this is a leak then one should take a look at what kind of GDI objects are
allocated (there are tools showing the details), find the lines in the source
where they are allocated, and try to figure out where they should be freed.

Comment 13

[ly nguyen]

some more stuff:

looks like imageWithAlphaBits can be accounted for although this line is funky:
380   mHBitmap = ::CreateDIBSection(TheHDC, (LPBITMAPINFO)&bmi, DIB_RGB_COLORS,
381                                 (LPVOID *)&imageWithAlphaBits, NULL, 0);

from google search, most people are calling CreateDIBSection like so:
HBITMAP hBmp = CreateDIBSection( NULL, pbmInfo, DIB_RGB_COLORS, &lpBits, 
						hSection, dwOffset );
where lpBits is just of type LPVOID not LPVOID*. *shrug* i'm sure it's nothing.
also, looking at some documentation for the return values for
CreateDIBSection(), i get this:
"Return Values
If the function succeeds, the return value is a handle to the newly created DIB,
and *[imageWithAlphaBits] points to the bitmap bit values.
If the function fails, the return value is NULL, and *[imageWithAlphaBits] is NULL."

so this tells me why imageWithAlphaBits is null. if mHBitmap is used
elsewhere...uh oh

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I'm wondering if the leak has something to do with how we get the appropriate
icons for all the file types from Windows.  (It also could have something to do
with the outliner image cache, but that seems less likely.  Although perhaps it
could be the interaction of the two.)

Comment 15

[Jaime Rodriguez, Jr.]

Nominating for nsbeta1, as this might the cause of other outliner tree widget
crashes.

Comment 16

[Bernard Alleysson]

Attachment: Fix for massive GDI Object leak

2 bitmaps and 1 icon were not destroyed for each file in the file listing

To verify the fix, use NT Task manager and watch GDI Objects count (comment
#3):
the count stays the same as you scroll or reload the
file:///c:/windows/system32 url

I don't know if this fixes the crash, it should be tested.

You can also see that there's a big memory leak when you reload the URL (>1MB
per reload) but this is another bug I think.

Comment 17

[ly nguyen]

i've patched my mozilla 1.0 build with the fix from above and it wfm.

Comment 18

[Shiva Thirumazhusai]

Attached Patch fixes the memory leak and also fixes the crash. Can you we get
r=/sr= going ?

Comment 19

[Kevin McCluskey (gone)]

Changing to ImageLib component and reassigning since the proposed fixed is in
ImageLib.

Comment 20

[Kevin McCluskey (gone)]

-> Reassigning

Comment 21

[Bernard Alleysson]

thx ly!

remarks:

this bug could be assigned to [e-mail] (cvsblame)

the crash occurs in GFX because mhHBitmap is not checked against NULL,
nsImageWin::CreateImageWithAlphaBits() have no return value maybe it would be
good to add an assertion (like ASSERT(mhBitmap != NULL) or something)

the allocations (GlobalAlloc() and nsImemory::Alloc() in
nsIconChannel::AsyncOpen() are not checked for failure, this could be done as
the file is touched

Comment 22

[Bernard Alleysson]

mkaply:
there is similar code for OS2 and I wonder if the bitmaps returned by
http://lxr.mozilla.org/seamonkey/source/modules/libpr0n/decoders/icon/os2/nsIconChannel.cpp#430
need to be freed too...

Comment 23

[tor]

The cleanup code changes look fine, but why did you remove the zeroing
of the bitmap structure (while adding a comment documenting the old
behavior)?

Comment 24

[Bernard Alleysson]

Zeroing is done by GlobalAlloc(GHND, ...)
GHND combines GMEM_MOVEABLE and *GMEM_ZEROINIT*, see
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/memory/memman_66qr.asp

Comment 25

[tor]

Comment on attachment 86516 [details] [diff] [review]
Fix for massive GDI Object leak

sr=tor

Comment 26

[Stuart Parmenter]

Comment on attachment 86516 [details] [diff] [review]
Fix for massive GDI Object leak

r=pavlov

Comment 27

[scottputterman]

getting this on adt and drivers radar.

Comment 28

[Shiva Thirumazhusai]

Bernard: Go ahead and check-in to the Trunk.

Comment 29

[Bernard Alleysson]

hmmm, I don't have CVS access
pavlov, could you check this in for me ?

Comment 30

[scottputterman]

adding adt1.0.1+.  Please get drivers approval before checking into the branch.

Comment 31

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 32

[Stuart Parmenter]

landed on the trunk and the branch.

Comment 33

[Terri Preston]

Verified fixed win XP branch build 2002062408, marking verified1.0.1

Comment 34

[Jay Patel [:jay]]

v.fixed per talkback data.  this crash is long gone.