Closed Bug 824266 Opened 11 years ago Closed 11 years ago

Crash in account manager when account has invalid server @ nsMsgAccountManager::GetIdentitiesForServer

Categories

(MailNews Core :: Account Manager, defect)

x86_64
Windows 7
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 20.0

People

(Reporter: rkent, Assigned: standard8)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

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.
Keywords: crash
Version: unspecified → Trunk
Blocks: 820377
Keywords: regression
Attached patch The fixSplinter Review
I think I took the check out as I thought it wasn't needed, but obviously I was wrong. This adds it back in.
Assignee: nobody → mbanner
Status: NEW → ASSIGNED
Attachment #695321 - Flags: review?(kent)
Severity: normal → critical
Comment on attachment 695321 [details] [diff] [review]
The fix

Thanks for fixing this.
Attachment #695321 - Flags: review?(kent) → review+
bp-8e73dd2d-99bc-402a-929f-ae8e32121217
nsMsgAccountManager::GetIdentitiesForServer(nsIMsgIncomingServer*, nsIArray**)
Crash Signature: [@ nsMsgAccountManager::GetIdentitiesForServer(nsIMsgIncomingServer*, nsIArray**) ]
Summary: Crash in account manager when account has invalid server → Crash in account manager when account has invalid server @ nsMsgAccountManager::GetIdentitiesForServer
https://hg.mozilla.org/comm-central/rev/17d78086de57
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 20.0
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: