Closed Bug 5742 Opened 21 years ago Closed 21 years ago

Crash -loading this page causes Apprunner crashes


(Core Graveyard :: Tracking, defect, P2, critical)



(Not tracked)



(Reporter: teruko, Assigned: peterl-retired)




(Whiteboard: DEPEND - Intl - Talkback ID 7949313 - peterl has fix.)

Tested 4-29-09 Win32 build.

This does not always happen, and this seems to happen under Winnt 4.0J.

Step of reproduce
1. Go to
2. Click on 8A or A1 for 936 code page, A1 for 950 code page
When the page is loading, Apprunner crashes.

I sent the talkback report, but now talkback does not work correctly.  So, I cannot find my incident.
Priority: P3 → P2
Target Milestone: M5
QA Contact: 3853 → 3851
Whiteboard: Talkback ID 7949313
Assignee: ftang → peterl
Summary: Crash Winnt4.0J -Loading this page causes Apprunner crashes → Crash -loading this page causes Apprunner crashes
It crash on nsHTMLDocument::InsertStyleSheetAt
line "      observer->StyleSheetAdded(this, aSheet);" where observer is 00000
stack trace-

nsHTMLDocument::InsertStyleSheetAt(nsHTMLDocument * const 0x0323b1c8,
nsIStyleSheet * 0x03238520, int 0x00000000, int 0x00000001) line 522 + 17 bytes
HTMLContentSink::LoadStyleSheet(nsIURL * 0x00f49e30, nsIUnicharInputStream *
0x03238ef0, int 0x00000001, const nsString & {...}, const nsString & {...},
nsIHTMLContent * 0x00f4988c, int 0x00000000) line 3138
nsDoneLoadingStyle(nsIUnicharStreamLoader * 0x00f49b20, nsString & {...}, void *
0x00f499d0, unsigned int 0x00000000) line 2188 + 54 bytes
nsUnicharStreamLoader::OnStopBinding(nsUnicharStreamLoader * const 0x00f49b24,
nsIURL * 0x00f49e30, unsigned int 0x00000000, unsigned short * 0x00f4fde0) line
156 + 31 bytes
nsDocumentBindInfo::OnStopBinding(nsDocumentBindInfo * const 0x00f4af50, nsIURL
* 0x00f49e30, unsigned int 0x00000000, unsigned short * 0x00f4fde0) line 2128 +
30 bytes
OnStopBindingProxyEvent::HandleEvent(OnStopBindingProxyEvent * const 0x00f4fbf0)
line 591 + 45 bytes
StreamListenerProxyEvent::HandlePLEvent(PLEvent * 0x00f4fbf4) line 471 + 12
PL_HandleEvent(PLEvent * 0x00f4fbf4) line 476 + 10 bytes
PL_ProcessPendingEvents(PLEventQueue * 0x00c209a0) line 437 + 9 bytes
_md_EventReceiverProc(void * 0x00a70338, unsigned int 0x0000c117, unsigned int
0x00000000, long 0x00c209a0) line 799 + 9 bytes
USER32! 77e5111a()

-	this	0x0323b1c8
+	nsMarkupDocument	{...}

-	mObservers	{...}
	mArray	0x00f4b8d0
	mArraySize	0x00000008
	mCount	0x00000001
+	observer	0x00000000
+	this	0x0323b1c8
+	aSheet	0x03238520
	aIndex	0x00000000
	aNotify	0x00000001
	index	0x00000001
	enabled	0x00000001
	count	0x00000002

Notice that the

+	nsIHTMLDocument	{...}
+	nsIDOMHTMLDocument	{...}
+	nsIDOMNSHTMLDocument	{...}
+	nsIHTMLContentContainer	{...}
+	mParentStack	0x0323b310
+	mChildStack	0x0323b3c0
	mStackInx	0x00000000
+	mSearchStr	0x00000000
	mSearchDirection	0xcdcdcdcd
	mLastBlockSearchOffset	0x00000000
	mAdjustToEnd	0x00000000
+	mHoldBlockContent	0x00000000
+	mBodyContent	0x00000000
	mShouldMatchCase	0x00000000
	mIsPreTag	0x00000000
+	mAttrStyleSheet	0x0323b4f0
+	mStyleAttrStyleSheet	0x0323b5e0
+	mBaseURL	0x00000000
+	mBaseTarget	0x00000000
	mDTDMode	eDTDMode_NoQuirks
+	mImageMaps	{...}
+	mImages	0x00000000
+	mApplets	0x00000000
+	mEmbeds	0x00000000
+	mLinks	0x00000000
+	mAnchors	0x00000000
+	mForms	0x00000000
+	mNamedItems	0x00000000
+	mParser	0x00000000
	mIsWriting	0x00000000
	mWriteLevel	0x00000000

Notice that count is 2 but mObservers.mCount is 1. I think this code is not
thread safe.... Some other thread could remove one observer from mObserver...

This bug cannot be reproduce every time by the procedure above. However, I did
reproduce this several times. I believe the trick to trigger it is the timing.
Just try to click several cell several times and you should be able to crash

Reassign this to peterl since he is the last one who touch the function
Note- Although the gif and the content of that page are related to Chinese,
there are no chinese text in the page.
It's not an issue of thread safety. It's simply that the doc observer was
removing itself as an observer as a result of the notification. There are a
handful of places in nsDocument where that's the case. I have a fix here...
did this get handed off before peterl left for europe?
Whiteboard: Talkback ID 7949313 → Talkback ID 7949313 - peterl has fix.
I tried both of the link.
build:  1999042908

they seem to work ok.
No. It got neither handed off nor fixed. I'm not sure this bug is really common
enough to for M5 consideration. Perhaps the reporter has more info there.

I'm in Amsterdam now (obviously I have connectivity) and I have the fix with me.
If wanted, I can put it in Monday night (here, late afternoon there), or someone
else familiar with nsDocument can easily do it (Troy, Kipp...). The fix is
simply to add logic to the observer notification loop to test that the observer
wasn't removed as a result of the observation, if it was, then back up the index
by one. Also, gate the for loop on a call to mObservers.Count() instead of the
cached value. (The same logic is done in a half-dozen places in nsDocument.cpp,
there are other places that need fixing too, but those can likely wait... I have
them fixed here along with other changes to documents)
Target Milestone: M5 → M6
Closed: 21 years ago
Resolution: --- → FIXED
I verified this in 5-19 and 5-20 Win32 build. under Winnt 4.0J and Windows 95J.
Whiteboard: Talkback ID 7949313 - peterl has fix. → DEPEND - Intl - Talkback ID 7949313 - peterl has fix.
Blocks: 7228
Moving all Apprunner bugs past and present to Other component temporarily whilst
don and I set correct component.  Apprunner component will be deleted/retired
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.