837643 - crash in nsMsgDatabase::ClearHdrCache

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[Wayne Mery (:wsmwk)]

This bug was filed from the Socorro interface and is 
report bp-888d8ea1-aff2-4afb-a247-f44b22130202 .
============================================================= 

0		@0xba6adc7	
1	xul.dll	nsMsgDatabase::ClearHdrCache	mailnews/db/msgdb/src/nsMsgDatabase.cpp:608
2	xul.dll	nsMsgDatabase::AddHdrToCache	mailnews/db/msgdb/src/nsMsgDatabase.cpp:465
3	xul.dll	nsMsgDatabase::CreateMsgHdr	mailnews/db/msgdb/src/nsMsgDatabase.cpp:788
4	xul.dll	nsMsgDBEnumerator::PrefetchNext	mailnews/db/msgdb/src/nsMsgDatabase.cpp:2834
5	xul.dll	nsMsgDBEnumerator::HasMoreElements	mailnews/db/msgdb/src/nsMsgDatabase.cpp:2860
6	xul.dll	nsMsgDatabase::InitRefHash	mailnews/db/msgdb/src/nsMsgDatabase.cpp:4245
7	xul.dll	nsMsgDatabase::GetRefFromHash	mailnews/db/msgdb/src/nsMsgDatabase.cpp:4113
8	xul.dll	nsMsgDatabase::GetThreadForMessageId	mailnews/db/msgdb/src/nsMsgDatabase.cpp:4431
9	xul.dll	nsMsgDatabase::ThreadNewHdr	mailnews/db/msgdb/src/nsMsgDatabase.cpp:4521
10	xul.dll	nsMsgDatabase::AddNewHdrToDB	mailnews/db/msgdb/src/nsMsgDatabase.cpp:3400
11	xul.dll	nsImapMailDatabase::AddNewHdrToDB	mailnews/db/msgdb/src/nsImapMailDatabase.cpp:104 

crashes last line of 
mwu@9538
601nsresult nsMsgDatabase::ClearHdrCache(bool reInit)
hg@0
602{
hg@0
603 if (m_cachedHeaders)
hg@0
604 {
hg@0
605 // save this away in case we renter this code.
hg@0
606 PLDHashTable *saveCachedHeaders = m_cachedHeaders;
mconley@13475
607 m_cachedHeaders = nullptr;
mconley@13475
608 PL_DHashTableEnumerate(saveCachedHeaders, HeaderEnumerator, nullptr); 

where those lines changed with mconley's checking with Bug 776630 - Switch comm-central from using nsnull to nullptr.  Prior to TB17 this crash sig was rare.

OTOH, slightly different stack with signature 0x0 | nsMsgDatabase::ClearHdrCache(bool) is not so rare in TB16
example - bp-63c6d147-d3ea-4aef-8b3f-d9ea52121203

ditto PL_DHashTableEnumerate | nsMsgDatabase::ClearHdrCache(bool) 
bp-3a10ed8b-b377-44df-b1be-ae9612130131

Comment 1

[Wayne Mery (:wsmwk)]

one more story similar to DHashTableEnumerate | nsMsgDatabase::ClearHdrCache(bool) -- well established in TB16

nsMsgDatabase::HeaderEnumerator(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*) 
bp-854efc23-9a80-47e5-88c0-0b5f82130204
0	xul.dll	nsMsgDatabase::HeaderEnumerator	mailnews/db/msgdb/src/nsMsgDatabase.cpp:486
1	xul.dll	PL_DHashTableEnumerate	objdir-tb/mozilla/xpcom/build/pldhash.cpp:715
2	xul.dll	nsMsgDatabase::ClearHdrCache	mailnews/db/msgdb/src/nsMsgDatabase.cpp:608
3	xul.dll	nsMsgDatabase::AddHdrToCache	mailnews/db/msgdb/src/nsMsgDatabase.cpp:465
4	xul.dll	nsMsgDatabase::CreateMsgHdr	mailnews/db/msgdb/src/nsMsgDatabase.cpp:788
5	xul.dll	nsMsgDatabase::GetMsgHdrForGMMsgID	mailnews/db/msgdb/src/nsMsgDatabase.cpp:4629
6	xul.dll	nsImapMailFolder::GetOfflineMsgFolder	mailnews/imap/src/nsImapMailFolder.cpp:9672
7	xul.dll	nsImapMailFolder::HasMsgOffline	mailnews/imap/src/nsImapMailFolder.cpp:9577
8	xul.dll	nsImapProtocol::TryToRunUrlLocally	mailnews/imap/src/nsImapProtocol.cpp:2026 

perhaps there is more than one bug.
and perhaps this is related to an even older crash?  bug 625850?

Comment 2

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

ClearHeaderEnumerator a few lines down also protects against null.

Comment 3

[:Irving Reid (No longer working on Firefox)]

Comment on attachment 731621 [details] [diff] [review]
proposed fix

Review of attachment 731621 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/db/msgdb/src/nsMsgDatabase.cpp
@@ +484,5 @@
>  {
>  
>    MsgHdrHashElement* element = reinterpret_cast<MsgHdrHashElement*>(hdr);
> +  if (element)
> +    NS_IF_RELEASE(element->mHdr);

This only gets called from http://mxr.mozilla.org/comm-central/source/mailnews/db/msgdb/src/nsMsgDatabase.cpp#603; any idea why m_cachedHeaders would contain a null entry?

That said, the fix looks safe to me.

Comment 4

[Magnus Melin [:mkmelin]]

(In reply to :Irving Reid from comment #3)
> nsMsgDatabase.cpp#603; any idea why m_cachedHeaders would contain a null
> entry?

No idea, sorry.

Comment 5

[Magnus Melin [:mkmelin]]

http://hg.mozilla.org/comm-central/rev/125f9c2a7a67 -> FIXED