Closed Bug 5742 Opened 26 years ago Closed 26 years ago

Crash -loading this page causes Apprunner crashes

Categories

(Core Graveyard :: Tracking, defect, P2)

x86
Other
defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: teruko, Assigned: peterl-retired)

References

()

Details

(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 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.
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 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.
Status: NEW → ASSIGNED
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
Status: ASSIGNED → RESOLVED
Closed: 26 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
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 shortly.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.