95688 - N610 crash [@ NS_MakeAbsoluteURI]

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[greer]

Users are reporting lots of crashes at the following stack below. Most crashes 
are happening on Win98 systems. May user are crashing trying to open mail. 
Needs reproducible steps.

   9 Windows 95  4.0 build 67109814
  10 Windows 95  4.0 build 67109975
   6 Windows 95  4.0 build 67306684
  32 Windows 98  4.10 build 67766222
  46 Windows 98  4.10 build 67766446
  16 Windows 98  4.90 build 73010104 (WinME)
   2 Windows NT  4.0 build 1381
   5 Windows NT  5.0 build 2195 (Win 2K Pro)


Stack Trace: 

         NS_MakeAbsoluteURI     [..\..\..\..\dist\include\nsNetUtil.h  line 228]
         nsHTMLFrameInnerFrame::DoLoadURL
[d:\builds\seamonkey\mozilla\layout\html\document\src\nsFrameFrame.cpp  line
1185]
         nsHTMLFrameInnerFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\document\src\nsFrameFrame.cpp  line
1292]
         nsContainerFrame::ReflowChild
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp  line
747]
         nsHTMLFrameOuterFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\document\src\nsFrameFrame.cpp  line
466]
         nsBoxToBlockAdaptor::Reflow
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxToBlockAdaptor.cpp  line
869]
         nsBoxToBlockAdaptor::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxToBlockAdaptor.cpp  line
525]
         nsBox::Layout  
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp  line
985]
         nsSprocketLayout::Layout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp  line 421]
         nsContainerBox::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp  line 553]
         nsBoxFrame::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 979]
         nsBox::Layout  
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp  line
985]
         nsSprocketLayout::Layout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp  line 421]
         nsContainerBox::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp  line 553]
         nsBoxFrame::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 979]
         nsBox::Layout  
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp  line
985]
         nsSprocketLayout::Layout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsSprocketLayout.cpp  line 421]
         nsContainerBox::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp  line 553]
         nsBoxFrame::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 979]
         nsBox::Layout  
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp  line
985]
         nsStackLayout::Layout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsStackLayout.cpp  line 256]
         nsContainerBox::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsContainerBox.cpp  line 553]
         nsBoxFrame::DoLayout
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 979]
         nsBox::Layout  
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBox.cpp  line
985]
         nsBoxFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsBoxFrame.cpp  line 781]
         nsRootBoxFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsRootBoxFrame.cpp  line 215]
         nsContainerFrame::ReflowChild
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsContainerFrame.cpp  line
747]
         ViewportFrame::Reflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsViewportFrame.cpp  line 538]
         PresShell::InitialReflow
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 2599]
         nsXULDocument::StartLayout
[d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULDocument.cpp  line
3918]
         nsXULDocument::ResumeWalk
[d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULDocument.cpp  line
5016]
         nsXULDocument::EndLoad
[d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULDocument.cpp  line
1586]
         XULContentSinkImpl::DidBuildModel
[d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULContentSink.cpp  line
513]
         CWellFormedDTD::DidBuildModel
[d:\builds\seamonkey\mozilla\htmlparser\src\nsWellFormedDTD.cpp  line 318]
         nsParser::DidBuildModel
[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp  line 1615]
         nsParser::ResumeParse
[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp  line 2143]
         nsParser::OnStopRequest
[d:\builds\seamonkey\mozilla\htmlparser\src\nsParser.cpp  line 2737]
         nsJARChannel::OnStopRequest
[d:\builds\seamonkey\mozilla\netwerk\protocol\jar\src\nsJARChannel.cpp  line
587]
         nsOnStopRequestEvent::HandleEvent
[d:\builds\seamonkey\mozilla\netwerk\base\src\nsRequestObserverProxy.cpp  line
161]
         PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  
line 591]
         PL_ProcessPendingEvents        
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c 
line 524]
         _md_EventReceiverProc  
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c 
line 1072]
 
        Source File : ../../../../dist/include/nsNetUtil.h line : 228

Comment 1

[greer]

Attachment: Talkback user comments

Comment 2

[karnaze (gone)]

Attachment: null check that may fix the bug

Comment 3

[Alexandru Savulov]

r= Alex Savulov; checking for null pointers is always a good ideea

Comment 4

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

Hmmm.  nsIDocument.h says it may return null, so what *should* callers do when
it does?  There are other places (e.g., nsStyleUtil.cpp) that don't check for
null.  Should we fix those too?  (Were all the NS_MakeAbsoluteURI stack traces
coming from nsFrameFrame?)

Comment 5

[kinmoz]

I just spoke with karnaze via phone ... according to the stack trace in this
bug, we are crashing when trying to free specstr:


inline nsresult
NS_MakeAbsoluteURI(nsAWritableString& result,
                   const nsAReadableString& spec,
                   nsIURI* baseURI = nsnull,
                   nsIIOService* ioService = nsnull)     // pass in nsIIOService
 to optimize callers
{
    char* resultStr;
    char* specStr = ToNewUTF8String(spec);
    if (!specStr) {
        return NS_ERROR_OUT_OF_MEMORY;
    }
    nsresult rv = NS_MakeAbsoluteURI(&resultStr, specStr, baseURI, ioService);
    nsMemory::Free(specStr);


that would lead me to believe there was something wrong with the spec arg passed
 in from nsFrameFrame, not the actual baseURI arg.

Comment 6

[karnaze (gone)]

kin, reassigning to you, as we agreed.

Comment 7

[kinmoz]

To answer dbaron's question, I did a query on all the talkback reports that
crashed in NS_MakeAbsoluteURI, I looked at about 40 or so of them (out of about
400+) and all of them were crashing as a result of nsFrameFrame::DoLoadURL()
calling NS_MakeAbsoluteURI(), no entry points from CSS that I saw.

The more I look at this stuff the more it looks like the line numbers in the
talkback reports are off by one (compared to the 0.9.2 and TRUNK source), so I
think karnaze's patch looking for baseURL == null was the correct thing.

I spent some time trying to figure out what situations nsDocument::GetBaseURL()
would return null, but had no luck, even using several of the URLs in the
talkback reports, as well as launching mail compose, and mail.

Giving back to karnaze so he can get credit for the fix.

sr=[e-mail]

Comment 8

[Asa Dotzler [:asa]]

kin, karnaze, does this belong in 0.9.4? 

(kin, also can you note your sr in the attachment manager by clicking Edit in
the Actions column of the Attachment table and checking the has-super-review
checkbox? thanks.)

Comment 9

[karnaze (gone)]

I checked the patch into the trunk and m0.9.2 branch. I'll ask drivers for 
approval on the m0.9.4 branch.

kin, thanks for the credit and the exceptionaly thorough sr.

Comment 10

[chris hofmann]

a=chofmann for the 0.9.4 branch

Comment 11

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

Line numbers for talkback reports on Windows are always off-by-one (or, rather,
the next non-empty line, or something of that sort).

Comment 12

[karnaze (gone)]

checked into m0.9.4 branch. fixed???

Comment 13

[Jan Carpenter]

This stack trace is not showing at all on the trunk talkback topcrash reports
(still on N610) 

Comment 14

[Chris Petersen]

Based on the previous comments, marking verified in the Oct 20th builds.