Teruko Kobayashi Reporter
	Description • 26 years ago

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 http://www.microsoft.com/globaldev/reference/dbcs/936.htm
   or http://www.microsoft.com/globaldev/reference/dbcs/950.htm
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.

	Frank Tang
	Comment 1 • 26 years ago

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
bytes
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
there.

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.

	Peter Linss Assignee
	Comment 2 • 26 years ago

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...

	chris hofmann
	Comment 3 • 26 years ago

did this get handed off before peterl left for europe?

	Zaw Oo
	Comment 4 • 26 years ago

I tried both of the link.
build:  1999042908

they seem to work ok.

	Peter Linss Assignee
	Comment 5 • 26 years ago

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)

	Teruko Kobayashi Reporter
	Comment 6 • 26 years ago

I verified this in 5-19 and 5-20 Win32 build. under Winnt 4.0J and Windows 95J.

	leger
	Comment 7 • 25 years ago

Moving all Apprunner bugs past and present to Other component temporarily whilst
don and I set correct component.  Apprunner component will be deleted/retired
shortly.