Closed
Bug 169036
Opened 23 years ago
Closed 22 years ago
Browser crashes [@ nsXULDocument::GetPrincipal] N700
Categories
(Core :: XUL, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: greer, Assigned: sicking)
Details
(Keywords: crash, qawanted, topcrash-)
Crash Data
Attachments
(1 file, 1 obsolete file)
1.54 KB,
patch
|
jrgmorrison
:
review+
jst
:
superreview+
dbaron
:
approval1.3+
|
Details | Diff | Splinter Review |
The talkback data is showing a lot of crashes at nsXULDocument::GetPrincipal.
All of the crashes are happening in the N7.0 data, but the code shows up in
MozillaTrunk. I don't see another bug for this. Is this a known issue?
From the registers it looks like |this| is null:
01d59111 8b442404 mov eax,[esp+0x4]
01d59115 ff742408 push dword ptr [esp+0x8]
01d59119 8b80dc010000 mov eax,[eax+0x1dc]
01d5911f 50 push eax
01d59120 8b08 mov ecx,[eax] <==Crashes here
01d59122 ff513c call dword ptr [ecx+0x3c]
01d59125 c20800 ret 0x8
Stack Trace:
nsXULDocument::GetPrincipal
[d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULDocument.cpp line 846]
nsNodeInfoManager::DropDocumentReference
[d:\builds\seamonkey\mozilla\content\base\src\nsNodeInfoManager.cpp line 181]
nsXULDocument::~nsXULDocument
[d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULDocument.cpp line 546]
nsXULDocument::`scalar deleting destructor'
nsXULDocument::Release
[d:\builds\seamonkey\mozilla\content\xul\document\src\nsXULDocument.cpp line 585]
nsCOMPtr_base::~nsCOMPtr_base
[d:\builds\seamonkey\mozilla\xpcom\glue\nsCOMPtr.cpp line 65]
nsContentDLF::CreateRDFDocument
[d:\builds\seamonkey\mozilla\content\build\nsContentDLF.cpp line 530]
nsContentDLF::CreateInstance
[d:\builds\seamonkey\mozilla\content\build\nsContentDLF.cpp line 280]
nsDocShell::NewContentViewerObj
[d:\builds\seamonkey\mozilla\docshell\base\nsDocShell.cpp line 4176]
nsDocShell::CreateContentViewer
[d:\builds\seamonkey\mozilla\docshell\base\nsDocShell.cpp line 4059]
nsDSURIContentListener::DoContent
[d:\builds\seamonkey\mozilla\docshell\base\nsDSURIContentListener.cpp line 108]
nsDocumentOpenInfo::DispatchContent
[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp line 365]
nsDocumentOpenInfo::OnStartRequest
[d:\builds\seamonkey\mozilla\uriloader\base\nsURILoader.cpp line 229]
nsCachedChromeChannel::HandleStartLoadEvent
[d:\builds\seamonkey\mozilla\rdf\chrome\src\nsChromeProtocolHandler.cpp line 451]
PL_HandleEvent [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c line 597]
PL_ProcessPendingEvents [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c
line 530]
_md_EventReceiverProc [d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c
line 1078]
KERNEL32.DLL + 0x24a97 (0xbff84a97)
0x006a83fa
0x00058f64
Source File :
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xul/document/src/nsXULDocument.cpp
line : 846
Comment 1•23 years ago
|
||
This ought to be reassigned from hyatt, probably. Any candidates or takers?
/be
Summary: Browser crashes [@ nsXULDocument::GetPrincipal] N700 → Browser crashes [@ nsXULDocument::GetPrincipal] N700
Comment 2•23 years ago
|
||
I think that this crash came about with the fix for bug 156452. In
~nsXULDocument, _after_ gRefCnt is 0, we do:
if (mNodeInfoManager) {
mNodeInfoManager->DropDocumentReference();
}
The fix for bug 156452 added code to nsNodeInfoManager::DropDocumentReference
that makes it try to access the dying nsXULDocument's GetPrincipal. But I
have no idea what the right thing to do instead.
Assignee | ||
Comment 3•23 years ago
|
||
i'll have a look
Comment 4•23 years ago
|
||
Oops. That's not quite a true statement (gRefCnt isn't necessarily 0 at this
point). Still, it is that code path that is being exercised.
Comment 6•22 years ago
|
||
This stack signature is not showing up in the talkback database in any builds
since 2002. Marking it fixed.
Reopen if you disagree.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 7•22 years ago
|
||
errr.. i thought we marked bugs fixed when they were fixed, not when people
stopped bumåing into them
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 8•22 years ago
|
||
Jonas: Yes...this should have not been marked fixed...but I think it might be
worth marking worksforme. Unless you think we still need to fix this somehow.
Talkback data doesn't show any crashes recently. I'll leave it up to you to
resolve this bug.
Assignee | ||
Comment 9•22 years ago
|
||
The only thing i could think of crashing is mMasterPrototype being null when
the nsXULDocument goes away and GetPrincipal is called.
I also moved up the mNodeInfoManager->DropDocumentReference call to the other
DropDocumentReference-calls just because it looked cleaner.
Assignee | ||
Comment 10•22 years ago
|
||
ok, this time with a *sane* number of contextlines
Attachment #114426 -
Attachment is obsolete: true
Assignee | ||
Updated•22 years ago
|
Attachment #114427 -
Flags: superreview?(jst)
Attachment #114427 -
Flags: review?(jrgm)
Comment 11•22 years ago
|
||
Comment on attachment 114427 [details] [diff] [review]
null-safety
sr=jst
Attachment #114427 -
Flags: superreview?(jst) → superreview+
Comment 12•22 years ago
|
||
The patch seems fine as it is, but I don't know... Do we really want to add
null check spackle to GetPrincipal when there isn't really much evidence of
this being a top, or even "middle" crash in trunk builds. [This is a common
crash on the branch, and would be really worth it there, but I only find one
incident of this crash on the trunk, and that was in a build from early
December]. nsXULDocument::GetPrincipal gets called a lot in the UI code.
Or maybe I'm just being a nitpicker :-)
Assignee | ||
Comment 13•22 years ago
|
||
I think we should add the nullcheck anyway. Right now we will crash if you
create a xul-document, set it's url and then bail (thus releasing it). In order
to be fully safe you have to set the masterprototype before setting the url,
which IMHO isn't a good requirement to have.
Even though this isn't a topcrasher any more i bet there are several cases where
we would crash if something failed at the "right" moment, for example running
out of memory. Better safe then sorry since i doubt that an extra |if| will be
noticeable perf-wise
Comment 14•22 years ago
|
||
Comment on attachment 114427 [details] [diff] [review]
null-safety
Okey-doke. r=jrgm
Attachment #114427 -
Flags: review?(jrgm) → review+
Assignee | ||
Comment 15•22 years ago
|
||
Comment on attachment 114427 [details] [diff] [review]
null-safety
fixes a potential crasher. This patch should be compleatly safe since all it
does is add a null-check.
Attachment #114427 -
Flags: approval1.3?
Attachment #114427 -
Flags: approval1.3? → approval1.3+
Assignee | ||
Comment 16•22 years ago
|
||
upps, i landed this a few days ago
Status: REOPENED → RESOLVED
Closed: 22 years ago → 22 years ago
Resolution: --- → FIXED
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: shrir → xptoolkit.widgets
Updated•14 years ago
|
Crash Signature: [@ nsXULDocument::GetPrincipal]
You need to log in
before you can comment on or make changes to this bug.
Description
•