221431 - crash: @nsHTMLDocument::ResolveName

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Darin Fisher]

crash: @nsHTMLDocument::ResolveName

see attached stack trace.  noticed this with CVS debug build from 2003-10-06.

steps to reproduce:

load www.cnn.com
load www.mozilla.org

press back and forward a bunch of times to switch between the two.  do so
quickly enough to prevent pages from completely loading.

stephend suggested that this might be a regression from harshal's HTML/XML
content sink merging.

Comment 1

[Darin Fisher]

Attachment: stack trace

Comment 2

[Darin Fisher]

in GDB, i'm getting this stack:

#0  0xdadadada in ?? ()
#1  0x4169f2e7 in nsHTMLDocument::FlushPendingNotifications(int, int)
(this=0x88612d8, aFlushReflows=0, aUpdateViews=0)
    at /builds/moz-trunk/mozilla/content/html/document/src/nsHTMLDocument.cpp:1528
#2  0x416a7779 in nsHTMLDocument::ResolveName(nsAString const&,
nsIDOMHTMLFormElement*, nsISupports**) (this=0x88612d8,
    aName=@0xbfff978c, aForm=0x0, aResult=0xbfff982c) at
/builds/moz-trunk/mozilla/content/html/document/src/nsHTMLDocument.cpp:3658
#3  0x42ed6c6f in nsHTMLDocumentSH::ResolveImpl(JSContext*,
nsIXPConnectWrappedNative*, long, nsISupports**) (cx=0x82c6710,
    wrapper=0x8893bc0, id=135361012, result=0xbfff982c) at
/builds/moz-trunk/mozilla/dom/src/base/nsDOMClassInfo.cpp:5198
#4  0x42ed711a in nsHTMLDocumentSH::NewResolve(nsIXPConnectWrappedNative*,
JSContext*, JSObject*, long, unsigned, JSObject**, int*) (
    this=0x83cfec8, wrapper=0x8893bc0, cx=0x82c6710, obj=0x883d628,
id=135361012, flags=1, objp=0xbfff98c0, _retval=0xbfff98c4)
    at /builds/moz-trunk/mozilla/dom/src/base/nsDOMClassInfo.cpp:5262
...

GDB is not being particularly helpful otherwise.  %eax has the value 0xdadadada,
and we are executing the ASM instruction "call *%eax" when the SEGV happens.  it
seems that the nsIParser instance (mParser) is corrupt or already free'd.

Comment 3

[Brendan Eich [:brendan]]

0xda is the dead-arena (freed arena memory) pattern.  Does that help?

/be

Comment 4

[Harshal Pradhan]

If it is an already free'd mParser, then this is very likely to be my regression.

Comment 5

[Harshal Pradhan]

Well, it is a use-after-free of the parser. But it doesn't look like I regressed
this. 

- I can reproduce fairly reliably with my current windows trunk debug build,
though my stack is sometimes different from darin's.
- I can reproduce in this build build even after I entirely back out
bug 218837.
- I happen to have the tree that I checked in bug 218837 from (Its not been
updated since). I can't reproduce in there despite trying rather hard.

So, it looks like this is something that regressed _after_ my contentsink checkins.

I managed to reproduce this under purify with a trunk build with bug 218837
entirely backed out. 

[E] FMR: Free memory read in nsParser::DidBuildModel(UINT) {1 occurrence}
    Reading 4 bytes from 0x0b4764bc (4 bytes at 0x0b4764bc illegal)
    Address 0x0b4764bc is 20 bytes into a 204 byte block at 0x0b4764a8
    Address 0x0b4764bc points to a C++ new block in heap 0x024f0000
    Thread ID: 0x7f4
    Error location
    nsParser::DidBuildModel(UINT) [nsparser.cpp:1252]
    nsParser::Terminate(void) [nsparser.cpp:1338]
    nsHTMLDocument::StopDocumentLoad(void) [nshtmldocument.cpp:1033]
    DocumentViewerImpl::Stop(void) [nsdocumentviewer.cpp:1146]
            nsDocShell::SetupNewViewer(nsIContentViewer *) [nsdocshell.cpp:4777]
            nsDocShell::Embed(nsIContentViewer *,char const*,nsISupports *)
[nsdocshell.cpp:4141]
            nsDocShell::CreateContentViewer(char const*,nsIRequest
*,nsIStreamListener * *) [nsdocshell.cpp:4553]
            nsDSURIContentListener::DoContent(char const*,int,nsIRequest
*,nsIStreamListener * *,int *) [nsdsuricontentlistener.cpp:109]
            nsDocumentOpenInfo::DispatchContent(nsIRequest *,nsISupports *)
[nsuriloader.cpp:411]
            nsDocumentOpenInfo::OnStartRequest(nsIRequest *,nsISupports *)
[nsuriloader.cpp:227]
        Allocation location
            new(UINT)      [newop.cpp:10]
            nsParserConstructor [nsparsermodule.cpp:72]
            nsGenericFactory::CreateInstance(nsISupports *,nsID const&,void * *)
[nsgenericfactory.cpp:86]
            nsComponentManagerImpl::CreateInstance(nsID const&,nsISupports
*,nsID const&,void * *) [nscomponentmanager.cpp:1913]
            nsCreateInstanceByCID::()(nsID const&,void * *)const
[nscomponentmanagerutils.cpp:55]
            nsCOMPtr<nsIParser>::assign_from_helper(nsCOMPtr_helper const&,nsID
const&) [nscomptr.h:971]
            nsCOMPtr<nsIParser>::=(nsCOMPtr_helper const&) [nscomptr.h:593]
            nsHTMLDocument::StartDocumentLoad(char const*,nsIChannel
*,nsILoadGroup *,nsISupports *,nsIStreamListener * *,int,nsIContentSink *)
[nshtmldocument.cpp:849]
            nsContentDLF::CreateDocument(char const*,nsIChannel *,nsILoadGroup
*,nsISupports *,nsID const&,nsIStreamListener * *,nsIContentViewer * *)
[nscontentdlf.cpp:431]
            nsContentDLF::CreateInstance(char const*,nsIChannel *,nsILoadGroup
*,char const*,nsISupports *,nsISupports *,nsIStreamListener * *,nsIContentViewer
* *) [nscontentdlf.cpp:225]
        Free location
            memset         [gkparser.dll]
            nsParser::`scalar deleting destructor'(UINT) [gkparser.dll]
            nsParser::Release(void) [nsparser.cpp:359]
            nsCOMPtr<nsIParser>::~nsCOMPtr<nsIParser>(void) [nscomptr.h:480]
            HTMLContentSink::DidBuildModel(void) [nshtmlcontentsink.cpp:2550]
                    RemoveDummyParserRequest();
                  }
                
             =>   return NS_OK;
                }
                
                NS_IMETHODIMP
            CNavDTD::DidBuildModel(UINT,int,nsIParser *,nsIContentSink *)
[cnavdtd.cpp:704]
            nsParser::DidBuildModel(UINT) [nsparser.cpp:1249]
            nsParser::Terminate(void) [nsparser.cpp:1338]
            nsHTMLDocument::StopDocumentLoad(void) [nshtmldocument.cpp:1033]
            DocumentViewerImpl::Stop(void) [nsdocumentviewer.cpp:1146]
[E] IPR: Invalid pointer read in
nsCOMPtr<nsIRequest>::assign_assuming_AddRef(nsIRequest *) {1 occurrence}

-----------------------------------------------------------------------------

I will investigate further, but its unlikely that I will be able to get back to
this before the next weekend. If someone else can deal with this in the
meantime, it will be much appreciated. 

Comment 6

[Harshal Pradhan]

I think I might have figured out what is going on here. Darin's changes in
nsParser.cpp from bug 210125 look like they may be causing extra releases of the
parser. 

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsParser.cpp&branch=&root=/cvsroot&subdir=mozilla/htmlparser/src&command=DIFF_FRAMESET&rev1=3.333&rev2=3.334

The release of the parser moved from HandlePLEvent to the nsParserContinueEvent()
dtor. Doesn't that mean that now we should not release the parser in 
nsParser::CancelParsingEvents()? (Presuming of coursethat we are never leaking
events etc.)

Comment 7

[Harshal Pradhan]

Attachment: Fix ?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132808 [details] [diff] [review]
Fix ?

Yeah, this looks reasonable.

Comment 9

[Darin Fisher]

Comment on attachment 132808 [details] [diff] [review]
Fix ?

harshal: doh!  thanks for discovering this.  sorry for thinking it was your
patch! :-(

Comment 10

[Darin Fisher]

fixed-on-trunk

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED.  I can no longer reproduce this crash using build 2004-06-30-08
on Windows XP with all steps given in this bug.