241864 - M18a2 Crash after trying to save page and visiting other url or reload current url - [@ ntdll.dll - ???] [@nsCOMPtr<nsIURI>::assign_assuming_AddRef]

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Martijn Wargers (dead)]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040421
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7b) Gecko/20040421

I did see the original problem here:
http://www.massagepraktijkdoorn.nl/
The above is a simplified/minimal testcase.

The following things must be used in the site to get the crash:
- There must be no filename in the url (not http://foo.com/index.html but
http://foo.com/)
- The index.html file has a weird title (with | and :: and spaces in it)
- The index.html file must be a frames page
- One of the framed pages (content2.html) must have a background-image (
background="content2_data/back2.gif") and that background-image must be in a
different directory.


Reproducible: Always
Steps to Reproduce:
1. Visit http://home.hccnet.nl/m.wargers/test/mozilla/crash/filesaveas5/
2. Try to save the page (doesn't seem to work)
3. Press reload or visit a differen site

Actual Results:  
Crash

Expected Results:  
No crash

Comment 1

[Martijn Wargers (dead)]

Talkback ID: TB31610Z

It can take a while before the actual crash occurs. Sometimes 20s or so. 

Comment 2

[u63580]

Can reproduce this in 1.7rc1 and FireFox 20040426  on Win2k.

Comment 3

[u63580]

Reproducable in 1.7rc1 and FireFox 20040426 on Win2k.  Related to bug 227830?

Comment 4

[Matthias Versen [:Matti]]

Stack Signature	 ntdll.dll + 0x4ca14 (0x77fcca14) a59b7930
Product ID	Mozilla17
Build ID	2004042109
Trigger Time	2004-04-27 04:48:38.0
Platform	Win32
Operating System	Windows NT 5.0 build 2195
Module	ntdll.dll + (0004ca14)
URL visited	http://home.hccnet.nl/m.wargers/test/mozilla/crash/filesaveas5/
User Comments	See bug 241864
Since Last Crash	sec
Total Uptime	sec
Trigger Reason	Access violation
Source File Name	
Trigger Line No.	
Stack Trace 
ntdll.dll + 0x4ca14 (0x77fcca14)
ntdll.dll + 0x4c774 (0x77fcc774)
MSVCRT.DLL + 0x1e00 (0x78001e00)
JS_free
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/js/src/jsapi.c, line 1483]
js_FinalizeObject
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/js/src/jsobj.c, line 2028]
js_GC [d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/js/src/jsgc.c,
line 1328]
js_ForceGC
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/js/src/jsgc.c, line 1001]
JS_GC [d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/js/src/jsapi.c,
line 1699]
nsJSContext::Notify
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1838]
nsTimerImpl::Fire
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpcom/threads/nsTimerImpl.cpp,
line 395]
nsAppShell::Run
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/widget/src/windows/nsAppShell.cpp,
line 142]
nsAppShellService::Run
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpfe/appshell/src/nsAppShellService.cpp,
line 524]
main1
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1313]
main
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1783]
WinMain
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1809]
WinMainCRTStartup()
KERNEL32.DLL + 0x11af6 (0x7c581af6)


Hmm, is this JS Engine, Events or something else ?

Comment 5

[Frank Wein [:mcsmurf]]

I think the stacktrace here is faulty, got the same with a opt build, but with a
debug build i already crash when i try to save the page (it crashes right after
i've selected the file where to save). I have also noticed if I choose a file
name like foo.html for saving, saving works fine (and doesn't crash with opt
build nor with debug build).
Here's the stacktrace with the debug build and when saving with the faulty(?!)
filename on Win2k with a current cvs trunk build and NTFS file system:
nsCOMPtr<nsIURI>::assign_assuming_AddRef(nsIURI * 0x0492b530) line 494 + 3 bytes
nsCOMPtr<nsIURI>::assign_with_AddRef(nsISupports * 0x0492b530) line 1023
nsCOMPtr<nsIURI>::operator=(const nsCOMPtr<nsIURI> & {...}) line 600
nsWebBrowserPersist::SaveSubframeContent(nsIDOMDocument * 0x047bd5f0, URIData *
0x0492bdc0) line 3300
nsWebBrowserPersist::OnWalkDOMNode(nsIDOMNode * 0x04634a20) line 2749
nsWebBrowserPersist::SaveDocumentInternal(nsIDOMDocument * 0x049265e0, nsIURI *
0x03defca8, nsIURI * 0x03de7780) line 1521
nsWebBrowserPersist::SaveDocument(nsWebBrowserPersist * const 0x049229f4,
nsIDOMDocument * 0x049265e0, nsISupports * 0x03defcac, nsISupports * 0x03e23568,
const char * 0x04701790, unsigned int 0x00002000, unsigned int 0x00000050) line
455 + 33 bytes
XPTC_InvokeByIndex(nsISupports * 0x049229f4, unsigned int 0x0000000a, unsigned
int 0x00000006, nsXPTCVariant * 0x0012e9b0) line 102
XPCWrappedNative::CallMethod(XPCCallContext & {...}, XPCWrappedNative::CallMode
CALL_METHOD) line 2027 + 43 bytes
XPC_WN_CallMethod(JSContext * 0x02438028, JSObject * 0x039806a8, unsigned int
0x00000006, long * 0x03d3f1a4, long * 0x0012ec80) line 1287 + 14 bytes
js_Invoke(JSContext * 0x02438028, unsigned int 0x00000006, unsigned int
0x00000000) line 1281 + 23 bytes
js_Interpret(JSContext * 0x02438028, long * 0x0012f6b4) line 3366 + 15 bytes
js_Invoke(JSContext * 0x02438028, unsigned int 0x00000003, unsigned int
0x00000002) line 1301 + 13 bytes
nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJSClass * const 0x04923500,
nsXPCWrappedJS * 0x03de0868, unsigned short 0x0004, const nsXPTMethodInfo *
0x02417ee0, nsXPTCMiniVariant * 0x0012fa00) line 1336 + 22 bytes
nsXPCWrappedJS::CallMethod(nsXPCWrappedJS * const 0x03de0868, unsigned short
0x0004, const nsXPTMethodInfo * 0x02417ee0, nsXPTCMiniVariant * 0x0012fa00) line 450
PrepareAndDispatch(nsXPTCStubBase * 0x03de0868, unsigned int 0x00000004,
unsigned int * 0x0012fab0, unsigned int * 0x0012faa0) line 117 + 31 bytes
SharedStub() line 147
nsURIChecker::SetStatusAndCallBack(unsigned int 0x00000000) line 86
nsURIChecker::OnStartRequest(nsURIChecker * const 0x03e1a2f4, nsIRequest *
0x03de82a0, nsISupports * 0x00000000) line 319
nsHttpChannel::CallOnStartRequest() line 668 + 60 bytes
nsHttpChannel::OnStartRequest(nsHttpChannel * const 0x03de82a8, nsIRequest *
0x046fc4c0, nsISupports * 0x00000000) line 3551
nsInputStreamPump::OnStateStart() line 378 + 42 bytes
nsInputStreamPump::OnInputStreamReady(nsInputStreamPump * const 0x046fc4c4,
nsIAsyncInputStream * 0x03973c4c) line 334 + 11 bytes
nsInputStreamReadyEvent::EventHandler(PLEvent * 0x03df1324) line 119
PL_HandleEvent(PLEvent * 0x03df1324) line 692 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00f17d98) line 627 + 9 bytes
_md_EventReceiverProc(HWND__ * 0x00070140, unsigned int 0x0000c11a, unsigned int
0x00000000, long 0x00f17d98) line 1433 + 9 bytes
241864

Comment 6

[Jay Patel [:jay]]

I was able to reproduce some sort of crash at
http://www.massagepraktijkdoorn.nl/...my stack looks a little different though:

Incident ID: 32197
Stack Signature	ntdll.dll + 0x33aed (0x77f83aed) 8e69b24d
Email Address	[e-mail]
Product ID	Mozilla17
Build ID	2004042109
Trigger Time	2004-04-27 16:38:35.0
Platform	Win32
Operating System	Windows NT 5.1 build 2600
Module	ntdll.dll + (00033aed)
URL visited	http://www.massagepraktijkdoorn.nl/
User Comments	loaded page, saved page as, refreshed page
Since Last Crash	sec
Total Uptime	sec
Trigger Reason	Access violation
Source File Name	
Trigger Line No.	
Stack Trace 	
ntdll.dll + 0x33aed (0x77f83aed)
ntdll.dll + 0x8cca (0x77f58cca)
msvcrt.dll + 0x1ab2e (0x77c2ab2e)
??3@YAXPAX@Z
nsChildContentList::`scalar deleting destructor'
nsHTMLScriptEventHandler::Release
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/html/content/src/nsHTMLScriptElement.cpp,
line 107]
nsDOMSlots::~nsDOMSlots
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsGenericElement.cpp,
line 751]
nsGenericElement::~nsGenericElement
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsGenericElement.cpp,
line 858]
nsHTMLImageElement::`scalar deleting destructor'
nsHTMLDListElement::Release
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/html/content/src/nsHTMLDListElement.cpp,
line 112]
nsAttrAndChildArray::Clear
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsAttrAndChildArray.cpp,
line 532]
nsAttrAndChildArray::~nsAttrAndChildArray
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsAttrAndChildArray.cpp,
line 77]
nsGenericElement::~nsGenericElement
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsGenericElement.cpp]
nsHTMLDivElement::`scalar deleting destructor'
nsHTMLDListElement::Release
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/html/content/src/nsHTMLDListElement.cpp,
line 112]
nsAttrAndChildArray::Clear
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsAttrAndChildArray.cpp,
line 532]
nsAttrAndChildArray::~nsAttrAndChildArray
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsAttrAndChildArray.cpp,
line 77]
nsGenericElement::~nsGenericElement
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsGenericElement.cpp]
nsHTMLDivElement::`scalar deleting destructor'
nsHTMLDListElement::Release
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/html/content/src/nsHTMLDListElement.cpp,
line 112]
nsAttrAndChildArray::Clear
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsAttrAndChildArray.cpp,
line 532]
nsAttrAndChildArray::~nsAttrAndChildArray
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsAttrAndChildArray.cpp,
line 77]
nsGenericElement::~nsGenericElement
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsGenericElement.cpp]
nsHTMLBodyElement::`scalar deleting destructor'
nsHTMLDListElement::Release
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/html/content/src/nsHTMLDListElement.cpp,
line 112]
nsAttrAndChildArray::Clear
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsAttrAndChildArray.cpp,
line 532]
nsAttrAndChildArray::~nsAttrAndChildArray
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsAttrAndChildArray.cpp,
line 77]
nsGenericElement::~nsGenericElement
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsGenericElement.cpp]
nsHTMLHtmlElement::`scalar deleting destructor'
nsHTMLDListElement::Release
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/html/content/src/nsHTMLDListElement.cpp,
line 112]
ReleaseObjects
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpcom/ds/nsCOMArray.cpp,
line 153]
nsVoidArray::EnumerateForwards
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpcom/ds/nsVoidArray.cpp,
line 652]
nsCOMArray_base::Clear
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/xpcom/ds/nsCOMArray.cpp,
line 160]
nsDocument::~nsDocument
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsDocument.cpp,
line 574]
nsDocument::Release
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/content/base/src/nsDocument.cpp,
line 668]
XPCJSRuntime::GCCallback
[d:/BUILDS/tinderbox/Mozilla1.7/WINNT_5.0_Clobber/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp,
line 549]
0x56077401

Confirming to NEW.  Adding M17rc1 to summary since I crashed with that milestone
and also putting in the topcrash keyword since this appears to be an easily
reproducible crash that others might be seeing (it's difficult to know for sure
because the stack signature is a .dll)

Comment 7

[Frank Wein [:mcsmurf]]

tweaking summary if someone trys with a debug build.
btw: In the console these warnings appear when clicking at File-Save Page As...
WARNING: malformed url: no scheme, file d:/mozilla/tree6/mozilla/netwerk/base/sr
c/nsStandardURL.cpp, line 705
spec=/_ Praktijk voor Natuurgeneeskunde en Massage Doorn   Klassieke Lichaamsmas
sage _ Chinese Massage _ Sportmassage _ Stoelmassage _ Natuurgeneeskunde _ Bindw
eefselmassage _ Holistic Pulsing _ Lymfedrainage _ Diepe Lichaamsmassage
WARNING: malformed url: no scheme, file d:/mozilla/tree6/mozilla/netwerk/base/sr
c/nsStandardURL.cpp, line 705

Comment 8

[Jay Patel [:jay]]

This is definitely still around in Mozilla 1.8a2.  I crashed using the urls in
this bug, but each stack is different (as the steps to the crash also varied
somewhat).

My incidents:
443197
443191
443221

Still, since we can't save pages like those described in comment #0 and the
steps here are easily reproducible, we should probably take a closer look here.
 Marking topcrash+.

Comment 9

[Jay Patel [:jay]]

This is definitely still around in Mozilla 1.8a2.  I crashed using the urls in
this bug, but each stack is different (as the steps to the crash also varied
somewhat).

My incidents:
443197
443191
443221

Still, since we can't save pages like those described in comment #0 and the
steps here are easily reproducible, we should probably take a closer look here.
 Marking topcrash+.

Comment 10

[Michael Lefevre]

I just crashed trying this testcase with a Firefox trunk build from 2004-12-23.
TB2766977Z

Comment 11

[Asa Dotzler [:asa]]

Darin, can you take a look at this for alpha6? 

Comment 12

[Darin Fisher]

Martijn: Your testcase appears to be down (resulting in a 404).  Would it be
possible for you to resurrect the testcase for us?  Thanks!!

Comment 13

[Martijn Wargers (dead)]

Oops! Ok, here it is again (this testcase can't be attached to bugzilla, that's
why it is external).
It crashes for me when I do a File->Save Page as, and then reload the same page
a few times.

Comment 14

[Asa Dotzler [:asa]]

Doesn't look like a fix is at hand. Hopefully Darin can look into this for beta. 

Comment 15

[Son Le]

The url in the steps to reproduce does not work (404 Not Found).

The actual testcase is in the URL text box under QA Contact, ie.
http://martijn.heelveel.info/test/mozilla/filesaveas5/

Comment 16

[Son Le]

Found out why this crash is occurring.

The problem is the site's long title and Windows' MAX_PATH limit of 248 chars.
When a page is saved, a directory is normally created with the same name as the
site (+ "_files" + frame name + "_data") to store all the images. In this case,
<site_name>_files\content2_data easily exceeds the limit.

The actual cause of this crash is an unchecked call to SaveDocumentInternal()
[on line 3362 in nsWebBrowserPersist.cpp] which then tries to save the data even
though the CreateDirectory call has failed.

A simple return check of SaveDocumentInternal() will prevent this crash, but the
page save will then fail silently.

Comment 17

[Son Le]

Attachment: patch v0

Check SaveDocumentInternal() return code patch.

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

great, thanks for tracking this down!

I see that this function sometimes returns NS_OK, sometimes PR_FALSE (both are
the same value, 0). since it's declared  nsresult, those should return rv / some
nsresult code...

Comment 19

[Son Le]

Should I make the changes as part of this bug? How about FixupURI(), which also
has the same problem?

Comment 20

[Boris Zbarsky [:bzbarsky]]

Yes, if you could make changes as part of this bug, that would be great.

Also, please make similar changes to FixupURI, and change things like:

  NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);

to

  NS_ENSURE_SUCCESS(rv, rv);

Comment 21

[Son Le]

Attachment: patch v1

Updated patch. Also changed an incorrect null-pointer check (!url).

Opened bug 281343 for MAX_PATH bug.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 173603 [details] [diff] [review]
patch v1

Looks reasonable

Comment 23

[Asa Dotzler [:asa]]

Darin, if you get free from 1.0.1 fixes, can you help with a review here? 

Comment 24

[Darin Fisher]

Comment on attachment 173603 [details] [diff] [review]
patch v1

it sucks that the compiler can't distinguish nsresult from PRBool.  sr=darin

Comment 25

[Asa Dotzler [:asa]]

Comment on attachment 173603 [details] [diff] [review]
patch v1

a=asa for checkin to 1.8b

Comment 26

[Darin Fisher]

fixed-on-trunk for 1.8b1

Comment 27

[Martijn Wargers (dead)]

Verified. Sorry, the url for my testcase changes again. (but should not be
necessary anymore :)