480556 - crash [@ nsMsgIdentity::getFolderPref(char const*, nsCString&, char const*, unsigned int)] - [@ nsMsgIdentity::GetArchiveFolder] on startup opening thunderbird

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Wayne Mery (:wsmwk)]

instant #1 topcrash hit for 3.0b2 
crash first appears build 20090223121634.
There are no crashes for 3.0b2pre.
please move bug to account/prefs if that's appropriate.

we're talking very big numbers, but since it's a crash on startup it might only be a few users. 3 crashes have comments, all 3 cite crash on first startup after installing 3.0b2. Can't tell if they ever get through startup after crashing oncest or twicest (PA dutch).

http://crash-stats.mozilla.com/report/list?product=Thunderbird&version=Thunderbird%3A3.0b2&version=Thunderbird%3A3.0b2pre&version=Thunderbird%3A3.0b3pre&query_search=signature&query_type=exact&query=&date=&range_value=10&range_unit=days&do_query=1&signature=nsMsgIdentity%3A%3AgetFolderPref%28char%20const*%2C%20nsCString%26%2C%20char%20const*%2C%20unsigned%20int%29

xref bug 330281, which has same top of stack, but rest of stack is not the same. Still, could be same crash and "archive" has just surfaced the issue.

bp-22f7b8c7-13a6-48c2-b169-2fd7b2090226
nsMsgIdentity::getFolderPref	nsMsgIdentity.cpp:343
nsMsgIdentity::GetArchiveFolder	nsMsgIdentity.cpp:264
nsMsgAccountManager::SetSpecialFolders	nsMsgAccountManager.cpp:1394
NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
XPCWrappedNative::CallMethod	js/src/xpconnect/src/xpcwrappednative.cpp:2424
XPC_WN_CallMethod	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1583

Comment 1

[David :Bienvenu]

It seems as if either server or root folder is null (Not sure how accurate the line numbers are). I would have thought we would have gotten an error before this, but perhaps not. I'll try to figure this out...

Comment 3

[(no longer active)]

I get this crash both on beta 3 and on the latest nightly.

Comment 4

[Wayne Mery (:wsmwk)]

(In reply to comment #2)
> *** Bug 480729 has been marked as a duplicate of this bug. ***

Ehsan, "I got this when upgrading from Beta 2 to Beta 3"

So, you are not able to get past startup at all, correct?d

I wonder if this also happens going direct from TB2 to beta 3

Comment 5

[David :Bienvenu]

Attachment: possible fix

Let's try bulletproofing this routine. I had fixed a case before where it looked like GetRootMsgFolder was returning success but a null folder, but that doesn't seem to have helped this case. So I'd like to try this to see if helps Ehsan.

Comment 6

[Mark Banner (:standard8)]

Looking at nsPop3IncomingServer::GetRootMsgFolder [1], it appears that nsMsgAccountManager::GetAccount [2] can return success with a null account, and that then means GetRootMsgFolder can also return null with success.

I'm thinking it may be better to fix it there else we'll need to consider all the other instances where GetRootMsgFolder is called.

[1] http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3IncomingServer.cpp#238
[2] http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgAccountManager.cpp#1514

Comment 7

[David :Bienvenu]

when I looked at this before, the other instances of GetRootMsgFolder checked for both null and error, so they don't have a problem. I'll fix nsPop3IncomingServer.cpp as well, but I think both fixes should go in.

Comment 8

[David :Bienvenu]

Attachment: cumulative patch

as you've suggested...

Comment 9

[Wayne Mery (:wsmwk)]

(In reply to comment #6)
> Looking at nsPop3IncomingServer::GetRootMsgFolder [1], it appears that
> nsMsgAccountManager::GetAccount [2] can return success with a null account, 

What constitutes a "null account"?  Would that be a rare thing to encounter?

Comment 10

[(no longer active)]

(In reply to comment #4)
> (In reply to comment #2)
> > *** Bug 480729 has been marked as a duplicate of this bug. ***
> 
> Ehsan, "I got this when upgrading from Beta 2 to Beta 3"
> 
> So, you are not able to get past startup at all, correct?d

That's correct.  The only thing that worked before the crash was the add-on manager checking my extensions and all.  The crash happened after the add-on manager's window was dismissed.

(In reply to comment #5)
> Let's try bulletproofing this routine. I had fixed a case before where it
> looked like GetRootMsgFolder was returning success but a null folder, but that
> doesn't seem to have helped this case. So I'd like to try this to see if helps
> Ehsan.

If you can create a try server build (or its equivalent for Thunderbird) with this patch, I'd be happy to test it to see if the crash is fixed.

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 364706 [details] [diff] [review]
cumulative patch

Yep, let's give this a try.

Comment 12

[David :Bienvenu]

fix checked in, Ehsan, you can try tomorrow's build - please let us know, thx!

Comment 13

[(no longer active)]

The crash doesn't happen any more with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b3pre) Gecko/20090301 Shredder/3.0b3pre.  Thanks for the quick fix!

Comment 14

[Wayne Mery (:wsmwk)]

I would still like to understand the two questions below.  And, ultimately, why didn't this appear in earlier testing?  It first shows with 3.0b2, yet archive bug 451995 and friends mostly landed well before this crash appears. Can we conclude we don't have many/enough pop users on trunk, or enough pop testers QAing the release?


(In reply to comment #9)
> (In reply to comment #6)
> > Looking at nsPop3IncomingServer::GetRootMsgFolder [1], it appears that
> > nsMsgAccountManager::GetAccount [2] can return success with a null account, 
> 
> What constitutes a "null account"?  Would that be a rare thing to encounter?

Comment 15

[David :Bienvenu]

the code that crashed only runs if the prefs for special folders aren't set, and it sets the prefs when successful. The code crashed when you had a slightly horked profile with a non-existent account, I think, which normally didn't cause any problems. Adding the Archives folder made this code run again for the archives folder pref, and crashed. We fixed an other bug with local folders archive prefs during the code freeze and that may have caused this crash for users with the borked profiles.

In general, I think the percentage of pop3 users of our nightly builds is much lower than the percentage of pop3 users of release builds.

Comment 17

[Wayne Mery (:wsmwk)]

fix signature for crash-stats