308328 - crash in [@ nsDocument::GetPrincipal] when closing browser

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050912 SeaMonkey/1.1a

crashed twice when closing some mail and browser windows.
Talkback TB9302104W
A search was showing 8 Talkbacks starting TrunkWin322005091105, 6 Seamonkey, 2
Firefox, the older ones are all Firefox and ended  Firefox10Win322005071605

Stacktrace is one line only, for all 8 records from different systems/people

Trigger Reason	Access violation
Source File, Line No.
c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/base/src/nsDocument.cpp,
line 1212
Stack Trace 	
nsDocument::GetPrincipal 
[c:/builds/tinderbox/MozillaTrunk/WINNT_5.0_Clobber/mozilla/content/base/src/nsDocument.cpp,
line 1212]

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

I think this is a regression from Bug 306363.

Comment 2

[Hermann Schwab]

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-09-10+04%3A00&maxdate=2005-09-11+06%3A00&cvsroot=%2Fcvsroot

Bug 306363  	Clean up nsContentUtils service getters
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/content/base/src&command=DIFF_FRAMESET&file=nsDocument.cpp&rev1=3.577&rev2=3.578&root=/cvsroot

Bug 106796  	Improve GetNameSpaceID() performance
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/content/base/src&command=DIFF_FRAMESET&file=nsDocument.cpp&rev1=3.578&rev2=3.579&root=/cvsroot

the crash:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/base/src/nsDocument.cpp&mark=1212&rev=#1212

Comment 3

[Hermann Schwab]

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=1&searchby=stacksig&match=contains&searchfor=nsDocument%3A%3AGetPrincipal&vendor=All&product=All&platform=All&buildid=&sdate=&stime=&edate=&etime=&sortby=bbid
first 19 are this regression, 5 are older (July).
Windows Talkbacks of Seamonkey, Firefox and Thunderbird Trunk are all the same
one-liner.

Linux Talkbacks are better:

http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=9349804
Stack Signature	nsDocument::GetPrincipal() 6bd15733
Product ID	ThunderbirdTrunk
Build ID	2005091107
Trigger Time	2005-09-14 11:36:05.0
Platform	LinuxIntel
OS      	Linux 2.6.9-11.ELsmp


http://talkback-public.mozilla.org/talkback/fastfind.jsp?search=2&type=iid&id=9339867
Stack Signature	 nsDocument::GetPrincipal() bc7cf8c7
Product ID	MozillaTrunk
Build ID	2005091307
Trigger Time	2005-09-14 05:41:37.0
Platform	LinuxIntel
Operating System	Linux 2.6.12.4
Module	libgklayout.so + (0024caf0)
URL visited	
User Comments	
Since Last Crash	0 sec
Total Uptime	0 sec
Trigger Reason	SIGSEGV: Segmentation Fault: (signal 11)
Source File, Line No.
/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/base/src/nsDocument.cpp,
line 842
Stack Trace 	
nsDocument::GetPrincipal() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/base/src/nsDocument.cpp,
line 842]
nsNodeInfoManager::DropDocumentReference() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/base/src/nsNodeInfoManager.cpp,
line 713]
nsDocument::~nsDocument() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/base/src/nsDocument.cpp,
line 792]
nsXMLDocument::~nsXMLDocument() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/xml/document/src/nsXMLDocument.cpp,
line 116]
nsDocument::Release() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/base/src/nsDocument.cpp,
line 871]
nsXMLDocument::Release() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/xml/document/src/nsXMLDocument.cpp,
line 203]
nsCOMPtr_base::~nsCOMPtr_base() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/xpcom/build/nsCOMPtr.cpp,
line 81]
nsXBLDocumentInfo::~nsXBLDocumentInfo() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/xbl/src/nsXBLDocumentInfo.cpp,
line 143]
nsXBLDocumentInfo::Release() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/xbl/src/nsXBLDocumentInfo.cpp,
line 350]
nsCOMPtr_base::~nsCOMPtr_base() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/xpcom/build/nsCOMPtr.cpp,
line 81]
nsBaseHashtableET<nsURIHashKey, nsCOMPtr<nsIXBLDocumentInfo>
>::~nsBaseHashtableET<nsURIHashKey, nsCOMPtr<nsIXBLDocumentInfo> >()
nsTHashtable<nsBaseHashtableET<nsURIHashKey, nsCOMPtr<nsIXBLDocumentInfo> >
>::s_ClearEntry()
PL_DHashTableFinish() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/xpcom/build/pldhash.c,
line 345]
nsTHashtable<nsBaseHashtableET<nsURIHashKey, nsCOMPtr<nsIXBLDocumentInfo> >
>::~nsTHashtable<nsBaseHashtableET<nsURIHashKey, nsCOMPtr<nsIXBLDocumentInfo> > >()
nsBindingManager::~nsBindingManager() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/xbl/src/nsBindingManager.cpp,
line 303]
nsBindingManager::Release() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/xbl/src/nsBindingManager.cpp,
line 298]
nsCOMPtr_base::~nsCOMPtr_base() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/xpcom/build/nsCOMPtr.cpp,
line 81]
nsDocument::~nsDocument() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/base/src/nsDocument.cpp,
line 465]
nsXMLDocument::~nsXMLDocument() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/xml/document/src/nsXMLDocument.cpp,
line 190]
nsXULDocument::~nsXULDocument() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/xul/document/src/nsXULDocument.cpp,
line 116]
nsDocument::Release() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/base/src/nsDocument.cpp,
line 871]
nsXMLDocument::Release() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/xml/document/src/nsXMLDocument.cpp,
line 203]
nsXULDocument::Release() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/content/xul/document/src/nsXULDocument.cpp,
line 476]
XPCJSRuntime::GCCallback() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp,
line 562]
DOMGCCallback() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 2212]
js_GC() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/js/src/jsgc.c,
line 1948]
js_ForceGC() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/js/src/jsgc.c,
line 1511]
js_DestroyContext() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/js/src/jscntxt.c,
line 284]
JS_DestroyContext() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/js/src/jsapi.c,
line 953]
mozJSComponentLoader::UnloadAll() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/js/src/xpconnect/loader/mozJSComponentLoader.cpp,
line 1008]
nsComponentManagerImpl::UnloadLibraries() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/xpcom/components/nsComponentManager.cpp,
line 3115]
nsComponentManagerImpl::Shutdown() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/xpcom/components/nsComponentManager.cpp,
line 900]
NS_ShutdownXPCOM_P() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/xpcom/build/nsXPComInit.cpp,
line 831]
main() 
[/builds/tinderbox/SeaMonkey-Release/Linux_2.4.20-28.8_Depend/mozilla/xpfe/bootstrap/nsAppRunner.cpp,
line 1753]
libc.so.6 + 0x1549f (0x4cb5949f)

Comment 4

[Peter Van der Beken [:peterv]]

So do we just want to revert the change from bug 306363 (and add plenty of null
checks)?
We could provide a way to detect that Shutdown has been called and use that to
guard certain calls to SecurityManager (for example by guarding the call to
GetPrincipal in nsNodeInfoManager::DropDocumentReference).
Or we could go back to GetSecurityManager and specify that it returns a
non-nsnull pointer, except after Shutdown has been called and only add the
null-checks in certain places.
I think I like the third one best. Any opinions?

Comment 5

[Hermann Schwab]

(In reply to comment #4)
> So do we just want to revert the change from bug 306363 (and add plenty of null
> checks)?

I hit this bug, looked at my TBid in fastfind, then searched for Stack signature
nsDocument::GetPrincipal and found only windows trunk builds starting
2005-SEP-11, and some others more than two months older.

The windows TBs are one-liners, the later coming Linux-TBs have a full stack
record, so I don't know if they are related to this bug.

Looking at the fastfind links into LXR, I assume that only one nullcheck must be
added. (When it is added, we may think about which to add next)
This one was easy, about 12 young records with same signature, and some
commented by others than me so I'm not alone with this issue.
It's been easy, because talkback didn`t show that signature for more than two
months, so I was assuming a 1 day regression based on the numbers.
There were two bugs maybe related, I couldn't decide in a minute, so I filed the
bug as long as the regression was fresh.
Comparing the links from comment 2 I assume the null-check removed here
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/content/base/src&command=DIFF_FRAMESET&file=nsDocument.cpp&rev1=3.577&rev2=3.578&root=/cvsroot
has produced the talkback TB9302104W and others on Windows, so so crashing here:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/content/base/src/nsDocument.cpp&mark=1212&rev=#1212


The Linux Talkbacks with complete stacks are looking different, but I don't know
more about Linux than about windows, so maybe they are the same.


> We could provide a way to detect that Shutdown has been called and use that to
> guard certain calls to SecurityManager 

As long as this isn't working maybe the (first) missing null-check should be
restored? You'll see where it crashes next.

Comment 6

[timeless]

i'm not sure which flavor is more useful,

Incident ID: 9405324
Stack Signature	nsDocument::GetPrincipal() 834ede90
Product ID	ThunderbirdTrunk
Build ID	2005091505
Trigger Time	2005-09-16 03:07:38.0
Platform	MacOSX
Operating System	Darwin 7.9.0
Module	thunderbird-bin + (002f6884)
URL visited	not canceling subscribing to a feed after quiting
User Comments	
Since Last Crash	480 sec
Total Uptime	480 sec
Trigger Reason	SIGBUS: Bus Error: (signal 10)
Source File, Line No.
/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/content/base/src/nsDocument.cpp,
line 842
Stack Trace 	
nsDocument::GetPrincipal() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/content/base/src/nsDocument.cpp,
line 842]
nsDocument::GetPrincipal() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/content/base/src/nsDocument.cpp,
line 899]
nsDocument::~nsDocument() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/content/base/src/nsDocument.cpp,
line 792]
nsXMLDocument::~nsXMLDocument() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/content/xml/document/src/nsXMLDocument.cpp,
line 190]
nsDocument::Release() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/content/base/src/nsDocument.cpp,
line 871]
nsXMLHttpRequest::~nsXMLHttpRequest()   nsXMLHttpRequest::Release()  
XPCJSRuntime::GCCallback() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/js/src/xpconnect/src/xpcjsruntime.cpp,
line 562]
DOMGCCallback() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 2209]
js_GC()  [/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/js/src/jsgc.c,
line 1948]
js_ForceGC() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/js/src/jsgc.c, line 1511]
js_DestroyContext() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/js/src/jscntxt.c, line 284]
mozJSComponentLoader::UnloadAll() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/js/src/xpconnect/loader/mozJSComponentLoader.cpp,
line 713]
nsComponentManagerImpl::UnloadLibraries() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/xpcom/components/nsComponentManager.cpp,
line 3115]
nsComponentManagerImpl::Shutdown() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/xpcom/components/nsComponentManager.cpp,
line 900]
NS_ShutdownXPCOM_P() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/xpcom/build/nsXPComInit.cpp,
line 831]
ScopedXPCOMStartup::~ScopedXPCOMStartup() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/toolkit/xre/nsAppRunner.cpp,
line 553]
XRE_main() 
[/builds/tinderbox/Tb-Trunk/Darwin_7.9.0_Depend/mozilla/toolkit/xre/nsAppRunner.cpp,
line 2349]
_start()   start()

Date/Time:      2005-09-16 03:07:30 -0700
OS Version:     10.3.9 (Build 7W98)
Report Version: 2

Command: thunderbird-bin
Path:    /Applications/Communications/Thunderbird
2.app/Contents/MacOS/thunderbird-bin
Version: 1.6a1 (1.6a1)
PID:     27983
Thread:  0

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_PROTECTION_FAILURE (0x0002) at 0x00000000

Thread 0 Crashed:
0   org.mozilla.thunderbird 	0x002f6884 nsDocument::GetPrincipal() + 0x40
1   org.mozilla.thunderbird 	0x006bafcc
nsNodeInfoManager::DropDocumentReference() + 0x40
2   org.mozilla.thunderbird 	0x002f4ebc _ZN10nsDocumentD4Ev + 0x33c
3   org.mozilla.thunderbird 	0x002ad078 _ZN13nsXMLDocumentD4Ev + 0xd8
4   org.mozilla.thunderbird 	0x002f5b2c nsDocument::Release() + 0x44
5   org.mozilla.thunderbird 	0x0045f700 _ZN16nsXMLHttpRequestD4Ev + 0xfc
6   org.mozilla.thunderbird 	0x0045fd3c nsXMLHttpRequest::Release() + 0x44
7   org.mozilla.thunderbird 	0x005472d8 XPCJSRuntime::GCCallback(JSContext*,
JSGCStatus) + 0x5fc
8   org.mozilla.thunderbird 	0x00381338 nsJSContext::FireGCTimer() + 0xfc
9   libmozjs.dylib          	0x0602b4b8 js_GC + 0xa90
10  libmozjs.dylib          	0x0602aa14 js_ForceGC + 0x40
11  libmozjs.dylib          	0x0600ea60 js_DestroyContext + 0x17c
12  org.mozilla.thunderbird 	0x0007bb54 mozJSComponentLoader::UnloadAll(int) + 0x94
13  libxpcom_core.dylib     	0x1004178c
nsComponentManagerImpl::UnloadLibraries(nsIServiceManager*, int) + 0x74
14  libxpcom_core.dylib     	0x1003dab0 nsComponentManagerImpl::Shutdown() + 0xa0
15  libxpcom_core.dylib     	0x10007910 NS_ShutdownXPCOM_P + 0x1d4
16  org.mozilla.thunderbird 	0x0000aa04 _ZN18ScopedXPCOMStartupD4Ev + 0x3c
17  org.mozilla.thunderbird 	0x0000ed64 XRE_main + 0xf88
18  org.mozilla.thunderbird 	0x0000a01c start + 0x1b0
19  org.mozilla.thunderbird 	0x00009e9c start + 0x30

I think my crash indicates that thunderbird leaked an xmlhttprequest or
something. from the looks of the other crashes here, it looks like there are
similar issues elsewhere. components that aren't listening to xpcom shutdown.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I also like option 3 the most.

Comment 8

[Peter Van der Beken [:peterv]]

Attachment: v1

Added back the null-check in the two places that we had it before.

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 198727 [details] [diff] [review]
v1

r+sr=bzbarsky, though I still think for XBL we want to just not fire stuff
during shutdown and remove that null-check...