Last Comment Bug 824266 - Crash in account manager when account has invalid server @ nsMsgAccountManager::GetIdentitiesForServer
: Crash in account manager when account has invalid server @ nsMsgAccountManage...
Status: RESOLVED FIXED
: crash, regression
Product: MailNews Core
Classification: Components
Component: Account Manager (show other bugs)
: Trunk
: x86_64 Windows 7
: -- critical (vote)
: Thunderbird 20.0
Assigned To: Mark Banner (:standard8, limited time in Dec)
:
:
Mentors:
Depends on:
Blocks: 820377
  Show dependency treegraph
 
Reported: 2012-12-22 07:50 PST by Kent James (:rkent)
Modified: 2012-12-30 02:54 PST (History)
4 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
The fix (610 bytes, patch)
2012-12-23 09:24 PST, Mark Banner (:standard8, limited time in Dec)
rkent: review+
Details | Diff | Splinter Review

Description Kent James (:rkent) 2012-12-22 07:50:24 PST
Bug 592710 introduced changes in account manager so that the internal m_accounts list would not eliminate accounts with invalid server, but instead just not use them. Bug 820377 did not take that into account, and introduced a new crash when an account with an invalid server is loaded.

Partial stack of the new crash:

>	xul.dll!nsMsgAccountManager::GetIdentitiesForServer(nsIMsgIncomingServer * server=0x05058080, nsIArray * * _retval=0x0038b978)  Line 2070	C++
 	xul.dll!nsMsgAccountManager::GetFirstIdentityForServer(nsIMsgIncomingServer * aServer=0x05058080, nsIMsgIdentity * * aIdentity=0x0038baac)  Line 2028 + 0x2a bytes	C++
 	xul.dll!nsMsgIncomingServer::ConfigureTemporaryReturnReceiptsFilter(nsIMsgFilterList * filterList=0x06c6d740)  Line 1955 + 0x41 bytes	C++
 	xul.dll!nsMsgIncomingServer::ConfigureTemporaryFilters(nsIMsgFilterList * aFilterList=0x06c6d740)  Line 1785 + 0xc bytes	C++
 	xul.dll!nsImapMailFolder::UpdateFolderWithListener(nsIMsgWindow * aMsgWindow=0x0501da60, nsIUrlListener * aUrlListener=0x00000000)  Line 662 + 0x31 bytes	C++
 	xul.dll!nsImapMailFolder::UpdateFolder(nsIMsgWindow * inMsgWindow=0x0501da60)  Line 624	C++
 	xul.dll!nsImapMailFolder::GetNewMessages(nsIMsgWindow * aWindow=0x0501da60, nsIUrlListener * aListener=0x00000000)  Line 2584 + 0x1d bytes	C++
 	xul.dll!nsImapIncomingServer::PerformBiff(nsIMsgWindow * aMsgWindow=0x0501da60)  Line 990 + 0x22 bytes	C++

Analysis:

Formerly, this check in nsMsgAccountManager::findIdentitiesForServer prevented using an account with an invalid server while looking for identities:

  if (!thisServer || !entry || !(entry->server))
    return true;

Such a check no longer exists, and a crash results. The new code instead uses this check:


   1.316 +    nsCOMPtr<nsIMsgIncomingServer> thisServer;
   1.317 +    rv = account->GetIncomingServer(getter_AddRefs(thisServer));
   1.318 +    if (NS_FAILED(rv))
   1.319 +      continue;
   1.320 +

but GetIncomingServer does not return a failed rv when there is no available server.
Comment 1 Mark Banner (:standard8, limited time in Dec) 2012-12-23 09:24:19 PST
Created attachment 695321 [details] [diff] [review]
The fix

I think I took the check out as I thought it wasn't needed, but obviously I was wrong. This adds it back in.
Comment 2 Kent James (:rkent) 2012-12-26 13:30:09 PST
Comment on attachment 695321 [details] [diff] [review]
The fix

Thanks for fixing this.
Comment 3 Wayne Mery (:wsmwk, NI for questions) 2012-12-27 04:51:35 PST
bp-8e73dd2d-99bc-402a-929f-ae8e32121217
nsMsgAccountManager::GetIdentitiesForServer(nsIMsgIncomingServer*, nsIArray**)
Comment 4 Mark Banner (:standard8, limited time in Dec) 2012-12-30 02:54:08 PST
https://hg.mozilla.org/comm-central/rev/17d78086de57

Note You need to log in before you can comment on or make changes to this bug.