Closed Bug 497875 Opened 11 years ago Closed 11 years ago
Crash [@ ns
XULDocument::Resume Walk][@ ns XULPrototype Document::Get URI() | ns XULDocument::Resume Walk] with loadoverlay and script
310 bytes, application/vnd.mozilla.xul+xml
8.18 KB, text/plain
24.25 KB, text/html
1.06 KB, patch
|Details | Diff | Splinter Review|
3.10 KB, patch
|Details | Diff | Splinter Review|
See testcase, which crashes current trunk build and Firefox3.0.x after the ftp site has bee loaded from the script tag. I'm not sure if this crashes online, or just locally. When testing locally, you need to have the security.fileuri.strict_origin_policy set to false to get the crash. http://crash-stats.mozilla.com/report/index/d56580ee-18bb-4aeb-84c0-738342090612?p=1 0 xul.dll nsXULDocument::ResumeWalk content/xul/document/src/nsXULDocument.cpp:2875 1 xul.dll nsXULDocument::OnStreamComplete content/xul/document/src/nsXULDocument.cpp:3472 2 xul.dll nsStreamLoader::OnStopRequest netwerk/base/src/nsStreamLoader.cpp:108 3 xul.dll nsBaseChannel::OnStopRequest netwerk/base/src/nsBaseChannel.cpp:680 4 xul.dll nsInputStreamPump::OnStateStop netwerk/base/src/nsInputStreamPump.cpp:576 5 xul.dll nsInputStreamPump::OnInputStreamReady netwerk/base/src/nsInputStreamPump.cpp:401 6 xul.dll nsOutputStreamReadyEvent::Run xpcom/io/nsStreamUtils.cpp:190 7 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:510 8 xul.dll nsBaseAppShell::Run widget/src/xpwidgets/nsBaseAppShell.cpp:170 9 xul.dll nsAppStartup::Run toolkit/components/startup/src/nsAppStartup.cpp:193 10 nspr4.dll PR_GetEnv 11 firefox.exe wmain toolkit/xre/nsWindowsWMain.cpp:110 12 firefox.exe firefox.exe@0x21a7 13 kernel32.dll kernel32.dll@0x17076 Firefox3.0.x crash report: http://crash-stats.mozilla.com/report/index/a22f72b9-c990-44f1-b2f2-ba8172090612?p=1 0 xul.dll nsXULDocument::ResumeWalk mozilla/content/xul/document/src/nsXULDocument.cpp:2822 1 xul.dll nsCOMPtr_base::assign_from_qi nsCOMPtr.cpp:98 2 xul.dll nsXULDocument::OnStreamComplete mozilla/content/xul/document/src/nsXULDocument.cpp:3371 Perhaps this is related to bug 475025?
Ok, the testcase doesn't crash online, so perhaps this isn't much of a security issue.
This is crashing in ResumeWalk because mCurrentPrototype is null.
Whiteboard: [sg:dos] null deref
Another crash I just got this morning with the same signature, but a slightly different stack: bp-972c2703-3e43-411f-8140-607992090902 I opened Minefield from Thunderbird to moderate a blog entry, something I'm doing on a daily basis, but have never seen crash before.
It looks like that this crash is the top crash #46 for Firefox 3.5.3. Even that only the first frame is identical but mCurrentPrototype seems to be always 0: http://crash-stats.mozilla.com/report/list?product=Firefox&version=Firefox%3A3.5.3&query_search=signature&query_type=exact&query=&date=&range_value=1&range_unit=weeks&do_query=1&signature=nsXULDocument%3A%3AResumeWalk%28%29 0 xul.dll nsXULDocument::ResumeWalk content/xul/document/src/nsXULDocument.cpp:2889 1 xul.dll nsCOMPtr_base::~nsCOMPtr_base obj-firefox/xpcom/build/nsCOMPtr.cpp:81 2 xul.dll xul.dll@0x334a7b A housemate informed me that it also happens for him when doing a search in the Google toolbar. Do we wanna handle the topcrasher in this bug or shall we better file a new one to address it?
The crashing line has been added by the patch on bug 419452. Probably a regression? I'll check. For now CC'ing Neil to this bug.
It's a regression which happens between the following builds: 2008-03-20-05-trunk/ 2008-03-21-06-trunk/ => Check-ins: http://tinyurl.com/m7qeqx 2008-03-26-02-mozilla-central/ 2008-03-27-02-mozilla-central/ => Check-ins: http://tinyurl.com/m7qeqx The m-c merge has a couple of the fixes from trunk. Olli, do you have an idea?
Same crash on OS X with another signature: bp-064322e3-f19c-40bc-89d6-637552090920 0 XUL nsXULPrototypeDocument::GetURI content/xul/document/src/nsXULPrototypeDocument.cpp:481 1 XUL nsXULDocument::ResumeWalk content/xul/document/src/nsXULDocument.cpp:2886 2 XUL nsXULDocument::OnStreamComplete content/xul/document/src/nsXULDocument.cpp:3609 3 XUL nsStreamLoader::OnStopRequest netwerk/base/src/nsStreamLoader.cpp:127 ...
OS: Windows XP → All
Hardware: x86 → All
Summary: Crash [@ nsXULDocument::ResumeWalk] with loadoverlay and script → Crash [@ nsXULPrototypeDocument::GetURI() | nsXULDocument::ResumeWalk] with loadoverlay and script
(In reply to comment #6) > It's a regression which happens between the following builds: > > 2008-03-20-05-trunk/ > 2008-03-21-06-trunk/ > => Check-ins: http://tinyurl.com/m7qeqx > > 2008-03-26-02-mozilla-central/ > 2008-03-27-02-mozilla-central/ > => Check-ins: http://tinyurl.com/m7qeqx The urls are the same. On purpose?
No. The correct second url would be: http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2008-03-26+00%3A00%3A00&enddate=2008-03-27+23%3A00%3A00
I can reproduce the crash on Linux when reloading the testcase. The problem appears to be that there is a nsFtpState that creates an async nsInputStreamReadyEvent which runs *after* we attempt to load the non-existing overlay. When we get the OnStreamComplete callback mCurrentPrototype is NULL. Failing to load an overlay sets mCurrentPrototype = null here: http://mxr.mozilla.org/mozilla-central/source/content/xul/document/src/nsXULDocument.cpp#2835
Do you mean the reason for the crash is that nsXULDocument::LoadOverlay tries (and fails, thus setting mCurrentPrototype to NULL) to load an overlay while there's a pending callback to ResumeWalk for the main document load? If that's the case, this is somewhat related to bug 330458. And your wallpaper is basically undoing the unconditional mCurrentPrototype access that was added in bug 419452: http://hg.mozilla.org/mozilla-central/diff/8d0008f4d372/content/xul/document/src/nsXULDocument.cpp
This probably should block on suspicion of being needed for bug 519767, which is a blocker. Neil, maybe you could help Mats here?
Assignee: nobody → matspal
(In reply to comment #14) > If that's the case, this is somewhat related to bug 330458. Yes, bug 330458 looks somewhat related. It would be great to have that fixed, but as roc noted, bug 519767 blocks 1.9.2 and we need to fix the crash for 1.9.2 with something less risky than what you have in bug 330458 (IMO). > And your wallpaper is basically undoing the unconditional > mCurrentPrototype access that was added in bug 419452 Sorry, but I don't see how it can break anything that bug 419452 fixed, if that's what you meant. If mCurrentPrototype is non-null the attached patch doesn't change anything. If it's null, the current code crashes, and from what I can see there's no harm in making |overlayURI| null there and continuing with the rest of the function. The aim of my patch is just to avoid the crash, not that it must be "correct" for all cases where |mCurrentPrototype| is null. Can you suggest a better fix? (that is still low-risk)
Comment on attachment 410514 [details] [diff] [review] wallpaper? Certainly in any test case using document.loadOverlay we will never actually use the URI, so it doesn't matter if we don't have one.
Attachment #410514 - Flags: review+
> Sorry, but I don't see how it can break anything that bug 419452 fixed, > if that's what you meant No, I meant the problem you're wallpapering over (assuming that mCurrentPrototype is always non-null in ResumeWalk) was introduced in that bug. Since this actually improves the confidence in the fix (at least for me), I wanted to note that, sorry about the confusion.
http://hg.mozilla.org/mozilla-central/rev/91f95d3d912e Sorry for the lack of regression test, but it's not possible to write crash tests that needs privileges currently (bug 448676), and the crash doesn't occur when loading the overlay over http (mochitest).
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
Can you write a mochichrome test?
Thanks for the suggestion. This crashes about 80% of the time without the patch.
mochichrome test: http://hg.mozilla.org/mozilla-central/rev/51417c7ae974
Flags: in-testsuite? → in-testsuite+
Comment on attachment 410514 [details] [diff] [review] wallpaper? Low-risk patch, like to get this in for beta2.
Attachment #410514 - Flags: approval1.9.2?
Attachment #410514 - Flags: approval1.9.2? → approval1.9.2+
Whiteboard: [sg:dos] null deref → [sg:dos] null deref [needs 192 landing]
Summary: Crash [@ nsXULPrototypeDocument::GetURI() | nsXULDocument::ResumeWalk] with loadoverlay and script → Crash [@ nsXULDocument::ResumeWalk][@ nsXULPrototypeDocument::GetURI() | nsXULDocument::ResumeWalk] with loadoverlay and script
Comment on attachment 410514 [details] [diff] [review] wallpaper? Approved for 184.108.40.206 and 220.127.116.11, a=dveditz for release-drivers
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f8f6ad616181 On CVS trunk: mozilla/content/xul/document/src/nsXULDocument.cpp 1.839 mozilla/content/xul/document/test/Makefile.in 1.8 mozilla/content/xul/document/test/bug497875-iframe.xul 1.1 mozilla/content/xul/document/test/test_bug497875.xul 1.1
Verified fixed on trunk, 1.9.2, and 1.9.1 with builds on Windows: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a1pre) Gecko/20091116 Minefield/3.7a1pre ID:20091116050530 Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2b4pre) Gecko/20091116 Namoroka/3.6b4pre ID:20091116045513 Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:18.104.22.168pre) Gecko/20091116 Shiretoko/3.5.6pre
Verified fixed for 22.214.171.124 using mochichrome test. Manual testcase does not crash 1.9.0 on Windows when tested.
Crash Signature: [@ nsXULDocument::ResumeWalk] [@ nsXULPrototypeDocument::GetURI() | nsXULDocument::ResumeWalk]
You need to log in before you can comment on or make changes to this bug.