128948 - Trunk crash [@ SINGSIGN_Enumerate]

Status: VERIFIED FIXED

(SeaMonkey :: Passwords & Permissions, defect)

Description

[Jay Patel [:jay]]

Talkback data is showing quite a few of these crashes with recent MozillaTrunk
builds on Linux and Windows.  Not sure if I got the correct component, but
although the user comments mention mail/news...the stack is pointing to the
wallet password manager.  Here is the latest data from Talkback:

SINGSIGN_Enumerate   30 
BBID range: 3244751 - 3571823
Min/Max Seconds since last crash: 10 - 80330
Min/Max Runtime: 35 - 247289
Crash data range: 2002-02-22 to 2002-03-02
Build ID range: 2002022113 to 2002030109
Keyword List : mail(5), news(5), 
Stack Trace: 

	 SINGSIGN_Enumerate
[d:\builds\seamonkey\mozilla\extensions\wallet\src\singsign.cpp  line 2660]
	 nsPasswordManagerEnumerator::GetNext
[d:\builds\seamonkey\mozilla\extensions\wallet\src\nsPasswordManager.cpp  line 69]
	 nsPasswordManager::FindPasswordEntry
[d:\builds\seamonkey\mozilla\extensions\wallet\src\nsPasswordManager.cpp  line 223]
	 nsMsgIncomingServer::GetIsAuthenticated
[d:\builds\seamonkey\mozilla\mailnews\base\util\nsMsgIncomingServer.cpp  line 1614]
	 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 2022]
	 XPC_WN_GetterSetter
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappednativejsops.cpp 
line 1299]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 790]
	 js_InternalInvoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 881]
	 js_Interpret
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 2596]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 806]
	 fun_apply
[d:\builds\seamonkey\mozilla\js\src\jsfun.c  line 1519]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 790]
	 js_Interpret
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 2746]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 806]
	 js_Interpret
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 2746]
	 js_Invoke
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c  line 806]
	 nsXPCWrappedJSClass::CallMethod
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappedjsclass.cpp  line 1195]
	 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]
	 nsEventListenerManager::HandleEventSubType
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line
1218]
	 nsEventListenerManager::HandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventListenerManager.cpp  line
2210]
	 nsXULElement::HandleDOMEvent
[d:\builds\seamonkey\mozilla\content\xul\content\src\nsXULElement.cpp  line 3457]
	 PresShell::HandleDOMEventWithTarget
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 6053]
	 nsButtonBoxFrame::MouseClicked
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsButtonBoxFrame.cpp  line 195]
	 nsButtonBoxFrame::HandleEvent
[d:\builds\seamonkey\mozilla\layout\xul\base\src\nsButtonBoxFrame.cpp  line 142]
	 PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 6022]
	 PresShell::HandleEventWithTarget
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5973]
	 nsEventStateManager::CheckForAndDispatchClick
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp  line 2588]
	 nsEventStateManager::PostHandleEvent
[d:\builds\seamonkey\mozilla\content\events\src\nsEventStateManager.cpp  line 1670]
	 PresShell::HandleEventInternal
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 6026]
	 PresShell::HandleEvent
[d:\builds\seamonkey\mozilla\layout\html\base\src\nsPresShell.cpp  line 5928]
	 nsViewManager::HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp  line 2043]
	 nsView::HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 306]
	 nsViewManager::DispatchEvent
[d:\builds\seamonkey\mozilla\view\src\nsViewManager.cpp  line 1863]
	 HandleEvent
[d:\builds\seamonkey\mozilla\view\src\nsView.cpp  line 83]
	 nsWindow::DispatchEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 869]
	 nsWindow::DispatchWindowEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 886]
	 nsWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 4715]
	 ChildWindow::DispatchMouseEvent
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 4965]
	 nsWindow::ProcessMessage
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 3632]
	 nsWindow::WindowProc
[d:\builds\seamonkey\mozilla\widget\src\windows\nsWindow.cpp  line 1131]
	 KERNEL32.DLL + 0x363b (0xbff7363b)
	 KERNEL32.DLL + 0x24407 (0xbff94407)
	 0x00648dee
 
 	Source File :
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/wallet/src/singsign.cpp
line : 2660
     (3377669)	Comments: In mailnews. clicked on Get all messages. The master secure
passworddialog came up (for no reason - none of my maill a/c passwords arestored
by mozilla) so I clicked cancel. Mozilla then crashed.
     (3303587)	URL: ANY news:// starts a wizard for A MAIL account..  :(
     (3303587)	Comments: ANY news:// starts a wizard for A MAIL account..  :(
     (3303523)	URL: ANY news:// starts a wizard for A MAIL account..  :(
     (3303523)	Comments: ANY news:// starts a wizard for A MAIL account..  :(
     (3274525)	Comments: getting all new messages
     (3244809)	Comments: again  attempting to enter mail/newsgroups after prior crash  am
prompted with account wizard  cancel  am prompted to enter password for software
security device  cancel  crash
     (3244751)	Comments: attempting to return to mail/newsgroups after prior crash  am
prompted with create account wizard  cancel  am prompted to enter password for
secure ??

I tried to reproduce it, but can't seem to get the dialog in question to pop up.
 Adding qawanted to see if we can get this reproduced.

Comment 1

[Jay Patel [:jay]]

Putting the right info in the right fields this time...sorry, brain fart!

Comment 2

[Stephen P. Morse]

I'm not sure yet where the problem lies, but this is obviously being triggered 
by the newly added method that racham inserted into nsPasswordManager.cpp.  
Specifically he added nsPasswordManager::FindPasswordEntry on February 12 as 
part of the fix for bug 80296.

I'd love to see a reproducible test case so I can determine whether the 
underlying cause is in this added code or is something that was lying dormant 
all along in the password manager and this added code simply brought it to 
light.  I'm still investigating.

Comment 3

[Peter Trudelle]

nsbeta1+ per Nav triage team

Comment 4

[racham]

Attachment: stack trace

In order to reproduce this bug, a master password setup (for those who are not
familiar with master password, please launch help system from app main menu
Help->Help and Support Center and click on search tab and type master password
and click on Go) is needed.

This bug could be reproduced by clicking on cancel button of master password
dialog which comes when user clicks on little dropmarker below GetMsg button
and select GetAllNewMessages in the mailnews window. 

Crash is coming as variable 'userStruct' in signsign.cpp is null in this case  


http://lxr.mozilla.org/mozilla/source/extensions/wallet/src/singsign.cpp#2660

Comment 5

[Stephen P. Morse]

Attachment: patch to break out if user fails to unlock the database

Comment 6

[Stephen P. Morse]

Attachment: same patch but with whitespace differences removed -- easier to review

Comment 7

[jag (Peter Annema)]

Comment on attachment 73400 [details] [diff] [review]
same patch but with whitespace differences removed -- easier to review

sr=jag

Comment 8

[racham]

Steve,

2 small changes.

1. Can you break if (NS_FAILED(rv)) return rv; into 2 lines. 
   if (NS_FAILED(rv)) 
     return rv;
   That will be useful debugging purposes in the future.

2. Also, The caller is doing NS_ENSURE_SUCCESS
(http://lxr.mozilla.org/seamonkey/source/mailnews/base/util/nsMsgIncomingServer.cpp#1612).
 Can you change that to 
   if (NS_FAILED(rv)) {
     // release hostURI
     nsMemory::Free(hostURI);
     return rv;
   }

We need to free the hostURI if we are going to bail out.

Do you have multiple account setup to test this one..?

thanks,
bhuvan (racham).

Comment 9

[Stephen P. Morse]

Attachment: patch that addresses racham's comments

Comment 10

[Stephen P. Morse]

Attachment: patch that addresses racham's comments but with whitespace differences ignored

Comment 11

[racham]

Comment on attachment 73535 [details] [diff] [review]
patch that addresses racham's comments but with whitespace differences ignored

r=bhuvan.

I did test on my local psm enabled build to make sure that it does the right
thing on GetAllNewMessages and does not crash on cancel.

Comment 12

[jag (Peter Annema)]

Comment on attachment 73535 [details] [diff] [review]
patch that addresses racham's comments but with whitespace differences ignored

sr=jag

Comment 13

[Asa Dotzler [:asa]]

Comment on attachment 73535 [details] [diff] [review]
patch that addresses racham's comments but with whitespace differences ignored

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 14

[Stephen P. Morse]

Fix checked in.

Comment 15

[Matthias Versen [:Matti]]

*** Bug 130401 has been marked as a duplicate of this bug. ***

Comment 16

[Terri Preston]

Verified patch checked into lxr.mozilla.org