642759 - crash [@ nsImapMailFolder::ApplyRetentionSettings()] - [@ nsMsgPurgeService::PerformPurge]

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Wayne Mery (:wsmwk)]

crash [@ nsImapMailFolder::ApplyRetentionSettings()] - [@ nsMsgPurgeService::PerformPurge]

bp-f0659d6d-cccb-437c-81b7-11fb42110313
EXCEPTION_ACCESS_VIOLATION_READ
0x0
0	thunderbird.exe	nsImapMailFolder::ApplyRetentionSettings	mailnews/imap/src/nsImapMailFolder.cpp:1307
1	thunderbird.exe	nsMsgPurgeService::PerformPurge	mailnews/base/src/nsMsgPurgeService.cpp:249
2	xpcom_core.dll	nsTimerImpl::Fire	xpcom/threads/nsTimerImpl.cpp:427
3	xpcom_core.dll	nsTimerEvent::Run	xpcom/threads/nsTimerImpl.cpp:519

Comment 1

[timeless]

652 nsresult nsImapMailFolder::GetDatabase()
653 {
654   nsresult rv = NS_OK;
655   if (!mDatabase)
656   {
657     nsCOMPtr<nsIMsgDBService> msgDBService = do_GetService(NS_MSGDB_SERVICE_CONTRACTID, &rv);
658     NS_ENSURE_SUCCESS(rv, rv);
659
660     // Create the database, blowing it away if it needs to be rebuilt
661     rv = msgDBService->OpenFolderDB(this, PR_FALSE, getter_AddRefs(mDatabase));
662     if (NS_FAILED(rv))
663       rv = msgDBService->CreateNewDB(this, getter_AddRefs(mDatabase));
664
665     NS_ENSURE_SUCCESS(rv, rv);
666
667     if(mDatabase)
668     {

this function seems to be as dangerous as UpdateSummaryTotals:
669       UpdateNewMessages();
670       if(mAddListener)
a smoking gun would be a crash on this line, but i'm not sure that we're likely to find it:
671         mDatabase->AddListener(this);
672       // UpdateSummaryTotals can null mDatabase during initialization, so we save a local copy
673       nsCOMPtr<nsIMsgDatabase> database(mDatabase);
674       UpdateSummaryTotals(PR_TRUE);
675       mDatabase = database;

Comment 2

[timeless]

Attachment: proposal

the initial null check is silly, since all paths ensure_success

Comment 3

[Wayne Mery (:wsmwk)]

timeless, thanks for the patch!

bienvenu wrote me "messing with this stuff is very scary - it's as likely to create problems as it is to fix them, so I need to make sure it's OK."

Comment 4

[David :Bienvenu]

Comment on attachment 520584 [details] [diff] [review]
proposal

I think this would only help if you switched the call to database->AddListener(PR_TRUE) instead of mDatabase. I can try running that change...

Comment 5

[David :Bienvenu]

Attachment: fix I landed

I landed this as - http://hg.mozilla.org/comm-central/rev/e0aafab0f5da

Comment 6

[David :Bienvenu]

speculatively marking fixed.