118137 - crash in nsMsgDBView code deleting large number of imap messages [@ nsUInt32Array::GetAt]

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect, P1)

Description

[David :Bienvenu]

nsUInt32Array::GetAt   8 
BBID range: 959653 - 1192996
Min/Max Seconds since last crash: 68 - 105451
Min/Max Runtime: 549 - 105451
Crash data range: 2001-12-27 to 2002-01-03
Build ID range: 2001122709 to 2002010110
Keyword List : 
Stack Trace: 

	 nsUInt32Array::GetAt
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsUInt32Array.cpp  line 144]
	 nsMsgDBView::OfflineMsgSelected
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsMsgDBView.cpp  line 4861]
	 nsMsgDBView::SelectionChanged
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsMsgDBView.cpp  line 886]
	 XPTC_InvokeByIndex
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp 
line 106]
	 XPCWrappedNative::CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp  line 2011]
	 XPC_WN_CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp 
line 1267]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 834]
	 js_Interpret
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 2799]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 850]
	 js_InternalInvoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 925]
	 JS_CallFunctionValue
[d:\builds\seamonkey\mozilla\js\src\jsapi.c  line 3407]
	 nsJSContext::CallEventHandler
[d:\builds\seamonkey\mozilla\dom\src\base\nsJSEnvironment.cpp  line 1014]
	 nsJSEventListener::HandleEvent
[d:\builds\seamonkey\mozilla\dom\src\events\nsJSEventListener.cpp  line 182]
	 nsEventListenerManager::HandleEventSubType
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line
1206]
	 nsEventListenerManager::HandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line
1809]
	 nsXULElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp  line 3449]
	 nsOutlinerSelection::FireOnSelectHandler
[d:\builds\seamonkey\mozilla\layout\xul\base\src\outliner\src\nsOutlinerSelection.cpp
 line 728]
	 nsOutlinerSelection::AdjustSelection
[d:\builds\seamonkey\mozilla\layout\xul\base\src\outliner\src\nsOutlinerSelection.cpp
 line 687]
	 nsOutlinerBodyFrame::RowCountChanged
[d:\builds\seamonkey\mozilla\layout\xul\base\src\outliner\src\nsOutlinerBodyFrame.cpp
 line 1279]
	 nsOutlinerBoxObject::RowCountChanged
[d:\builds\seamonkey\mozilla\layout\xul\base\src\outliner\src\nsOutlinerBoxObject.cpp
 line 371]
	 nsMsgDBView::NoteChange
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsMsgDBView.cpp  line 3971]
	 nsMsgDBView::OnDeleteCompleted
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsMsgDBView.cpp  line 4841]
	 XPTC_InvokeByIndex
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp 
line 106]
	 XPCWrappedNative::CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednative.cpp  line 2011]
	 XPC_WN_CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp 
line 1267]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 834]
	 js_Interpret
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 2799]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 850]
	 nsXPCWrappedJSClass::CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappedjsclass.cpp  line 1218]
	 nsXPCWrappedJS::CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappedjs.cpp  line 430]
	 PrepareAndDispatch
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp 
line 117]
	 SharedStub
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp 
line 139]
	 nsMsgMailSession::OnItemEvent
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsMsgMailSession.cpp  line 331]
	 nsMsgFolder::NotifyFolderEvent
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgFolder.cpp  line 2470]
	 nsImapMailFolder::OnStopRunningUrl
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapMailFolder.cpp  line 3942]
	 nsUrlListenerManager::BroadcastChange
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsUrlListenerManager.cpp  line 99]
	 nsUrlListenerManager::OnStopRunningUrl
[d:\builds\seamonkey\mozilla\mailnews\base\src\nsUrlListenerManager.cpp  line 128]
	 nsMsgMailNewsUrl::SetUrlState
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgMailNewsUrl.cpp  line 131]
	 nsImapMailFolder::SetUrlState
[d:\builds\seamonkey\mozilla\mailnews\imap\src\nsImapMailFolder.cpp  line 4661]
	 XPTC_InvokeByIndex
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp 
line 106]
	 EventHandler
[d:\builds\seamonkey\mozilla\xpcom\proxy\src\nsProxyEvent.cpp  line 516]
	 PL_HandleEvent
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line 591]
	 PL_ProcessPendingEvents
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line 524]
	 _md_EventReceiverProc
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c  line 1072]
	 nsAppShellService::Run
[d:\builds\seamonkey\mozilla\xpfe\appshell\src\nsAppShellService.cpp  line 303]
	 netscp6.exe + 0x167d (0x0040167d)
	 netscp6.exe + 0x121c (0x0040121c)
	 netscp6.exe + 0x3221 (0x00403221)
	 KERNEL32.DLL + 0x17d08 (0x77e97d08)
 
 	Source File :
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/base/util/nsUInt32Array.cpp
line : 144
     (1057741)	Comments: When deleting more that 20-30 emails Mozilla CRASHES ... This is
really annoying since I was trying to clean up the repeat filter mess that
Mozilla caused in the first place (The one that happens when it crashes like
this  and hasn't deleted (expunged)
     (1057741)	Comments:  the messages that were filtered to new mailboxes.
     (1045708)	Comments: Deleting messages using IMAP/SSL
     (959653)	Comments: Moving I-MAP e-mail

Comment 1

[David :Bienvenu]

this started happening right after the batch delete optimizations, so I suspect
they're somehow related, if only by accidentally exposing some other problem.
From the stack trace, I'd say that the selection and the flags array are out of
sync - somehow, we're being told things are selected that aren't valid indexes
in the flags array. One fix is to just bulletproof nsMsgDBView::OfflineMsgSelected
 to check for valid view indices, but that might just shift the crash to later
down stream. The thing to do is try to reproduce this problem, and figure out
why the selection indices are out of sync.

Comment 2

[(not reading, please use [e-mail] instead)]

I'm investigating #116174 right now, which looks very related.

Comment 3

[Navin Gupta]

I will try to reproduce. 

Comment 4

[Navin Gupta]

looking at climate there are lot of crashes here nsUint32Array::GetAt on other 
activities like compacting folders, receving pop mail, switching imap folders, 
reading mail, subscribing to newsgroup... 

Comment 5

[Navin Gupta]

I have no luck in reproducing this crash. cc'ing sheela, karen, if they
can help in reproducing this crash. 

Comment 6

[Navin Gupta]

cc 'ing ssaux from talkback report who saw this crash. if you remember
how many messages were being deleted, what kind of selection (shift, ctrl key)
number of messages in the folder, or any other info that will help us in 
reproducing it. 

Comment 7

[Stephane Saux]

I can't have been all that many emails. I usually delete my mails by either
clickin g on the icon or hitting the delete key.  I probably used the ctrl key
to select the messages.

One thing I think I remember is that I started the delete, then moved to another
(existing) window.  The crash happened shortly almost immediately after the
focus was given to the newly-focused window.

Comment 8

[(not reading, please use [e-mail] instead)]

I've checked in my fix for #116174, which might have helped this problem.

but given that this crasher is a top crasher, we should keep this open until we 
find a way to reproduce it, or it stops showing up as a top crasher.

Comment 9

[Navin Gupta]

This is not a topcrash because there are other crashes with same 
stack signature.

Comment 10

[Navin Gupta]

Just checked talkback, we are still crashing and I am able to reproduce the
crash finally. m_flags array is ok but indices[index] is garbage, that is 
why it crashes. 

Comment 11

[Navin Gupta]

Attachment: proposed fix

When we are notifying block removal of rows to the outliner - NoteChange
it calls adjustSelection and that ends up calling SelectionChanged. But if 
we are in the middle of deleting rows we do not have to bother about loading
next message because we end up calling selectionChanged when we select
msgToSelectAfterDelete. 

It was crashing because numSelected > num of indices selected so we were 
accessing indices out of bounds. I have added an assertion for that. 

david, please review.

Comment 12

[David :Bienvenu]

Comment on attachment 63997 [details] [diff] [review]
proposed fix

r=bienvenu, seems OK. Please run it past Seth as well. Thx.

Comment 13

[(not reading, please use [e-mail] instead)]

here's another approach:

nsOutlinerSelection::FireOnSelectHandler() fires the dom event that will result
in our SelectionChanged() being called.  instead of muting the event there,
let's suppress the firing.

use:

  // first, freeze selection.
  mOutlinerSelection->SetSelectEventsSuppressed(PR_TRUE);

and

  // unfreeze selection.
  mOutlinerSelection->SetSelectEventsSuppressed(PR_FALSE);

nsOutlinerSelection::FireOnSelectHandler() checks mSuppressed, and returns
immediately if we are suppressing.

your added assertion is still worthwhile.  can you try this approach instead?

Comment 14

[Navin Gupta]

Attachment: patch

Using SuppressEvent, this works as well and we execute even less code.
when we set it to false, it calls fireOnSelectHandler, which doesn't
do much but it is ok.

Comment 15

[(not reading, please use [e-mail] instead)]

Comment on attachment 64066 [details] [diff] [review]
patch

sr=sspitzer

looks good, assuming we don't return any time after we freeze, but before we
unfreeze.

Comment 16

[Navin Gupta]

fixed. should make multiple deletes faster!!

Comment 17

[David :Bienvenu]

re-opening, this caused bug 119202 - we might need to go with your first fix
instead.

Comment 18

[Navin Gupta]

So the problem was that when we set SetSuppressedEvent(FALSE) it 
updatedCommandStatus() {because there was no message selected at that time}
thereby disabling the toolbar. when we select nextMsg after delete
command updating is disabled as explained in this comment.


mscott 1.131 // if we are about to set the selection with a new element then 
DON'T clear 
 // the selection then add the next message to select. This just generates
 // an extra round of command updating notifications that we are trying to
 // optimize away.

Comment 19

[Navin Gupta]

Comment on attachment 63997 [details] [diff] [review]
proposed fix

this patch is the
way to go because
it doesn't allow
SelectionChnaged()
to be called, that
updates the command
status. seth, can
you sr.

Comment 20

[(not reading, please use [e-mail] instead)]

Comment on attachment 63997 [details] [diff] [review]
proposed fix

sr=sspitzer

sorry my suggestion caused a regression.

Comment 21

[Navin Gupta]

fixed. 

Comment 22

[Dawn Endico]

this fix works for me. I deleted my normal 20 spam messages this morning
with today's nightly build and did not crash.

Comment 23

[[e-mail] (gone - use [e-mail] instead)]

I just deleted 1,000 test messages in my IMAP account on build 2002-01-14-03 on
Windows 2000.

The last crash in this stack was with  2002-01-10-12   MozillaTrunk

verified fixed.