Last Comment Bug 324988 - Crash if stop loading while favicons are coming in
: Crash if stop loading while favicons are coming in
Status: RESOLVED FIXED
: crash, fixed1.8.0.7, fixed1.8.1
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
: P1 critical (vote)
: ---
Assigned To: Brian Ryner (not reading)
:
Mentors:
: 325343 (view as bug list)
Depends on:
Blocks: 256297
  Show dependency treegraph
 
Reported: 2006-01-27 15:36 PST by Brett Wilson
Modified: 2008-07-31 03:20 PDT (History)
5 users (show)
dveditz: blocking1.8.0.7+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
possible fix (5.66 KB, patch)
2006-01-28 21:05 PST, Brian Ryner (not reading)
no flags Details | Diff | Splinter Review
working fix (diff -w) (5.62 KB, patch)
2006-01-28 22:23 PST, Brian Ryner (not reading)
bzbarsky: review+
bzbarsky: superreview+
bzbarsky: approval‑branch‑1.8.1+
dveditz: approval1.8.0.7+
Details | Diff | Splinter Review

Description Brett Wilson 2006-01-27 15:36:56 PST
If you open the places page with a screenful of history and lots of favicons and navigate to another page before the icons have finished loading, you will get a segfault. Occurs on trunk and 1.8 branch. You probably have to do that after starting the browser so the images aren't cached. You might also need to be running on a networked drive on Linux to make sure the images come in slowly enough. I've found the best way to get this is to start the browser, go to some page, click on places, and quickly hit back.

Stack trace:
#0  CancelImageRequest (aKey=0x8a6ebe8, aData=0x8a6ea98, aClosure=0x0)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp:119
#1  0x401572e1 in hashEnumerate (table=0x8a6c458, hdr=0x0, i=0, arg=0x41a0da88)
    at /home/brettw/moz/branch/mozilla/xpcom/ds/nsHashtable.cpp:131
#2  0x40146a29 in PL_DHashTableEnumerate (table=0x8a6c458,
    etor=0x401572ca <hashEnumerate>, arg=0xbfffe4f0) at pldhash.c:621
#3  0x401579e5 in nsHashtable::Enumerate (this=0x8a6c450, aEnumFunc=0,
    aClosure=0x0)
    at /home/brettw/moz/branch/mozilla/xpcom/ds/nsHashtable.cpp:319
#4  0x4213ca73 in nsSupportsHashtable::Enumerate (this=0x8a6c450,
    aEnumFunc=0x4250bba4 <CancelImageRequest>, aClosure=0x0)
    at nsHashtable.h:221
#5  0x4250c45b in ~nsTreeBodyFrame (this=0x8963dcc)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp:173
#6  0x4217c9de in nsFrame::Destroy (this=0x8963dcc, aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsFrame.cpp:674
#7  0x4250ccf0 in nsTreeBodyFrame::Destroy (this=0x8963dcc,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp:366
#8  0x4218c931 in nsFrameList::DestroyFrames (this=0x8963cb8,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsFrameList.cpp:138
#9  0x42178a1d in nsContainerFrame::Destroy (this=0x8963c84,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsContainerFrame.cpp:161
#10 0x422c3680 in nsBoxFrame::Destroy (this=0x8963c84, aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/nsBoxFrame.cpp:1120
#11 0x4218c931 in nsFrameList::DestroyFrames (this=0x8963bf4,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsFrameList.cpp:138
#12 0x42178a1d in nsContainerFrame::Destroy (this=0x8963bc0,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsContainerFrame.cpp:161
#13 0x422c3680 in nsBoxFrame::Destroy (this=0x8963bc0, aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/nsBoxFrame.cpp:1120
#14 0x4218c931 in nsFrameList::DestroyFrames (this=0x89631e4,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsFrameList.cpp:138
#15 0x42178a1d in nsContainerFrame::Destroy (this=0x89631b0,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsContainerFrame.cpp:161
#16 0x422c3680 in nsBoxFrame::Destroy (this=0x89631b0, aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/nsBoxFrame.cpp:1120
#17 0x4218c931 in nsFrameList::DestroyFrames (this=0x8962ee4,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsFrameList.cpp:138
#18 0x42178a1d in nsContainerFrame::Destroy (this=0x8962eb0,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsContainerFrame.cpp:161
#19 0x422c3680 in nsBoxFrame::Destroy (this=0x8962eb0, aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/nsBoxFrame.cpp:1120
#20 0x4218c931 in nsFrameList::DestroyFrames (this=0x88d6358,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsFrameList.cpp:138
#21 0x42178a1d in nsContainerFrame::Destroy (this=0x88d6324,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsContainerFrame.cpp:161
#22 0x422c3680 in nsBoxFrame::Destroy (this=0x88d6324, aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/nsBoxFrame.cpp:1120
#23 0x4218c931 in nsFrameList::DestroyFrames (this=0x88d6268,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsFrameList.cpp:138
#24 0x42178a1d in nsContainerFrame::Destroy (this=0x88d6234,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsContainerFrame.cpp:161
#25 0x422c3680 in nsBoxFrame::Destroy (this=0x88d6234, aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/nsBoxFrame.cpp:1120
#26 0x4218c931 in nsFrameList::DestroyFrames (this=0x88c0b8c,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsFrameList.cpp:138
#27 0x42178a1d in nsContainerFrame::Destroy (this=0x88c0b58,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsContainerFrame.cpp:161
#28 0x422c3680 in nsBoxFrame::Destroy (this=0x88c0b58, aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/nsBoxFrame.cpp:1120
#29 0x4218c931 in nsFrameList::DestroyFrames (this=0x88c0828,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsFrameList.cpp:138
#30 0x42178a1d in nsContainerFrame::Destroy (this=0x88c07f4,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsContainerFrame.cpp:161
#31 0x422c3680 in nsBoxFrame::Destroy (this=0x88c07f4, aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/xul/base/src/nsBoxFrame.cpp:1120
#32 0x4218c931 in nsFrameList::DestroyFrames (this=0x88c0798,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsFrameList.cpp:138
#33 0x42178a1d in nsContainerFrame::Destroy (this=0x88c0764,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsContainerFrame.cpp:161
#34 0x421ee61c in ViewportFrame::Destroy (this=0x88c0764,
    aPresContext=0x88c9d70)
    at /home/brettw/moz/branch/mozilla/layout/generic/nsViewportFrame.cpp:67
#35 0x42131535 in nsFrameManager::Destroy (this=0x888eb24)
    at /home/brettw/moz/branch/mozilla/layout/base/nsFrameManager.cpp:297
#36 0x4213fb2d in PresShell::Destroy (this=0x888eb08)
    at /home/brettw/moz/branch/mozilla/layout/base/nsPresShell.cpp:1954
#37 0x4211f938 in DocumentViewerImpl::Destroy (this=0x86c0948)
    at /home/brettw/moz/branch/mozilla/layout/base/nsDocumentViewer.cpp:1437
#38 0x4212059b in DocumentViewerImpl::Show (this=0x88eadb0)
    at /home/brettw/moz/branch/mozilla/layout/base/nsDocumentViewer.cpp:1715
#39 0x4213bf7a in nsPresContext::EnsureVisible (this=0x8838ad0,
    aUnsuppressFocus=0)
    at /home/brettw/moz/branch/mozilla/layout/base/nsPresContext.cpp:1293
#40 0x421482f0 in PresShell::UnsuppressAndInvalidate (this=0x890d108)
    at /home/brettw/moz/branch/mozilla/layout/base/nsPresShell.cpp:5029
#41 0x4214c34e in PresShell::ProcessReflowCommands (this=0x890d108,
    aInterruptible=1)
    at /home/brettw/moz/branch/mozilla/layout/base/nsPresShell.cpp:6942
#42 0x4215617e in ReflowEvent::HandleEvent (this=0x890d108)
    at /home/brettw/moz/branch/mozilla/layout/base/nsPresShell.cpp:6702
#43 0x4214baf1 in HandlePLEvent (aEvent=0x89b4720)
    at /home/brettw/moz/branch/mozilla/layout/base/nsPresShell.cpp:6719
#44 0x401aaf0a in PL_HandleEvent (self=0x89b4720)
    at /home/brettw/moz/branch/mozilla/xpcom/threads/plevent.c:688
#45 0x401aade3 in PL_ProcessPendingEvents (self=0x80b1d18)
    at /home/brettw/moz/branch/mozilla/xpcom/threads/plevent.c:623
#46 0x401ad9a2 in nsEventQueueImpl::ProcessPendingEvents (this=0x80b7898)
    at /home/brettw/moz/branch/mozilla/xpcom/threads/nsEventQueue.cpp:417
#47 0x412f199e in event_processor_callback (source=0x809e750,
    condition=G_IO_IN, data=0x41a0da88)
    at /home/brettw/moz/branch/mozilla/widget/src/gtk2/nsAppShell.cpp:67
#48 0x4067bddf in g_vsnprintf () from /usr/lib/libglib-2.0.so.0
#49 0x4065ab35 in g_get_current_time () from /usr/lib/libglib-2.0.so.0
#50 0x4065bb78 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#51 0x4065be8d in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#52 0x4065c58f in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
#53 0x40382f5f in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
Comment 1 Brett Wilson 2006-01-27 15:37:41 PST
Oh yeah, that stack trace was on the 1.8 branch
Comment 2 Brian Ryner (not reading) 2006-01-28 16:48:18 PST
Moving from Firefox/Places to Core/Layout:XUL since this is a core XUL crash.
Comment 3 Brian Ryner (not reading) 2006-01-28 16:48:52 PST
(In reply to comment #2)
> Moving from Firefox/Places to Core/Layout:XUL since this is a core XUL crash.
> 

Or rather, XP Toolkit/XUL
Comment 4 Brian Ryner (not reading) 2006-01-28 18:15:47 PST
I think the reason it crashes is that we cancel the document's load group when loading a new page into the docshell, which cancels all of the image requests.  This happens before frame teardown, so the decoderObserver has already been nulled out by the time CancelImageRequest is called (see imgRequestProxy::Cancel).

We need to fix this without leaking the TreeImageListener objects, of course.  Since we need to keep 1 outstanding reference to the image listener, and the image request doesn't hold onto it long enough, maybe we should extend the image cache so that it maps from URL -> {request, listener}.

Another option would be to create a new loadgroup for these image requests, and add it to the document loadgroup.  As the loadgroup owner, the tree frame could observe OnStopRequest for the loadgroup and release the listeners as the requests are removed.  This works because the loadgroup observer is notified before the requests are actually cancelled.

Boris, any preference here, or other ideas?
Comment 5 Brian Ryner (not reading) 2006-01-28 21:05:17 PST
Created attachment 210019 [details] [diff] [review]
possible fix

This patch ensures that the listener is released when the corresponding request is removed from the image cache.  I switched over to nsDataHashtable, which makes the code a good deal cleaner (tree frames are now slightly larger, but I don't think that's a big deal).

Note: I haven't yet tested this to confirm that the crash is gone
Comment 6 Brian Ryner (not reading) 2006-01-28 22:19:14 PST
Comment on attachment 210019 [details] [diff] [review]
possible fix

This isn't quite right, the destructor gets invoked as temporary objects are created, so we end up cancelling the requests too early
Comment 7 Brian Ryner (not reading) 2006-01-28 22:23:16 PST
Created attachment 210025 [details] [diff] [review]
working fix (diff -w)

I'm no longer able to reproduce the crash with this patch.
Comment 8 Boris Zbarsky [:bz] 2006-01-29 21:33:51 PST
Comment on attachment 210025 [details] [diff] [review]
working fix (diff -w)

Oh, excellent!  Sorry about the mess the first time around.  :(  This is a vast improvement.

It might be worth getting this in on the 1.8.0 branch too.
Comment 9 Brian Ryner (not reading) 2006-01-30 12:34:51 PST
Comment on attachment 210025 [details] [diff] [review]
working fix (diff -w)

I'd like to get this on the branch since FF2 will be loading XUL files into the content area and thereby make this much easier to hit.
Comment 10 Brian Ryner (not reading) 2006-01-30 15:22:54 PST
checked in on trunk and branch
Comment 11 Andrew Schultz 2006-02-16 23:12:31 PST
*** Bug 325343 has been marked as a duplicate of this bug. ***
Comment 12 Daniel Veditz [:dveditz] 2006-08-15 14:18:00 PDT
Comment on attachment 210025 [details] [diff] [review]
working fix (diff -w)

may fix bug 256297
Comment 13 Daniel Veditz [:dveditz] 2006-08-15 14:56:19 PDT
Comment on attachment 210025 [details] [diff] [review]
working fix (diff -w)

approved for 1.8.0 branch, a=dveditz for drivers
Comment 14 Brian Ryner (not reading) 2006-08-22 22:43:08 PDT
checked in on 1.8.0 branch

Note You need to log in before you can comment on or make changes to this bug.