Closed Bug 726655 Opened 8 years ago Closed 6 months ago

crash nsMsgDBView::GetThreadContainingMsgHdr

Categories

(MailNews Core :: Backend, defect, critical)

x86
All
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 71.0

People

(Reporter: wsmwk, Assigned: jorgk-bmo)

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-2f6ac3c8-9b5f-4dd9-bfb1-927012120213 .
============================================================= 
only addon listed for this crash is is gconversation
0	xul.dll	nsMsgDBView::GetThreadContainingMsgHdr	mailnews/base/src/nsMsgDBView.cpp:5000
1	xul.dll	nsMsgGroupView::GetThreadContainingMsgHdr	mailnews/base/src/nsMsgGroupView.cpp:959
2	xul.dll	nsMsgDBView::ThreadIndexOfMsgHdr	mailnews/base/src/nsMsgDBView.cpp:4683
3	xul.dll	nsMsgDBView::FindIndexOfMsgHdr	mailnews/base/src/nsMsgDBView.cpp:7558
4	xul.dll	NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
Haven't seen this happen at all. There used to be an old, similar bug, but it got fixed months ago. Bienvenu might have some better insights...
m_db is null - probably just need to protect against this.
(In reply to David :Bienvenu from comment #2)
> m_db is null - probably just need to protect against this.

bp-14a4cbd0-e13c-4767-89ab-523f02121222
bp-e2921c30-81f0-46bc-9faa-769762121219
Crash Signature: [@ nsMsgDBView::GetThreadContainingMsgHdr(nsIMsgDBHdr*, nsIMsgThread**)] → [@ nsMsgDBView::GetThreadContainingMsgHdr(nsIMsgDBHdr*, nsIMsgThread**)] [@ nsMsgDBView::GetThreadContainingMsgHdr]
(In reply to David :Bienvenu from comment #2)
> m_db is null - probably just need to protect against this.

Do you agree?  Or will this only wallpaper a real issue that needs deeper investigation?

I see two types of stacks in recent examples 

* bp-054850d6-5732-4486-b928-30ff90171229  bp-e9cb9e2a-cdc3-49dd-b4e4-bf1670180613
0 	xul.dll	nsMsgDBView::GetThreadContainingMsgHdr(nsIMsgDBHdr*, nsIMsgThread**)
1 	xul.dll	nsMsgGroupView::GetThreadContainingMsgHdr(nsIMsgDBHdr*, nsIMsgThread**)
2 	xul.dll	nsMsgDBView::ThreadIndexOfMsgHdr(nsIMsgDBHdr*, unsigned int, int*, unsigned int*)
3 	xul.dll	nsMsgDBView::FindIndexOfMsgHdr(nsIMsgDBHdr*, bool, unsigned int*)
4 	xul.dll	NS_InvokeByIndex
5 	xul.dll	XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode)

* bp-3e0a9475-20db-46ea-91ff-b78b40180617
0 	xul.dll	nsMsgDBView::GetThreadContainingMsgHdr(nsIMsgDBHdr*, nsIMsgThread**)
1 	xul.dll	nsMsgGroupView::GetThreadContainingMsgHdr(nsIMsgDBHdr*, nsIMsgThread**)
2 	xul.dll	nsMsgDBView::AddHdr(nsIMsgDBHdr*, unsigned int*)
3 	xul.dll	nsMsgThreadedDBView::OnNewHeader(nsIMsgDBHdr*, unsigned int, bool)
4 	xul.dll	nsMsgDBView::OnHdrAdded(nsIMsgDBHdr*, unsigned int, int, nsIDBChangeListener*)
5 	xul.dll	nsMsgDatabase::NotifyHdrAddedAll(nsIMsgDBHdr*, unsigned int, int, nsIDBChangeListener*)
Flags: needinfo?(m_kato)
(In reply to Wayne Mery (:wsmwk) from comment #4)
> (In reply to David :Bienvenu from comment #2)
> > m_db is null - probably just need to protect against this.
> 
> Do you agree?  Or will this only wallpaper a real issue that needs deeper
> investigation?

Yes, this crash occurs since m_db is null.
Flags: needinfo?(m_kato)

Is it still the case that "m_db is null - probably just need to protect against this." ?

Perhaps coincidence, but 10 of 12 randomish crashes I examined have gconversations addon installed
bp-5c314ea8-6814-46a4-8db0-b40870190917 68.1.0

Crash Signature: [@ nsMsgDBView::GetThreadContainingMsgHdr(nsIMsgDBHdr*, nsIMsgThread**)] [@ nsMsgDBView::GetThreadContainingMsgHdr] → [@ nsMsgDBView::GetThreadContainingMsgHdr]
Flags: needinfo?(jorgk)
OS: Windows NT → All

Yes.

Alta88, I'll add if (!m_db) return XXXX here:
https://searchfox.org/comm-central/rev/266e9cc242cd0de076e85eb4aa0b8392fcb2ca01/mailnews/base/src/nsMsgDBView.cpp#4951

Can you pre-approve that and let me know which error?
In that same file we have:
if (key == nsMsgKey_None || !m_db) return NS_MSG_INVALID_DBVIEW_INDEX;
if (!m_db) return NS_MSG_MESSAGE_NOT_FOUND;
if (!m_db) return NS_ERROR_FAILURE;

Maybe just the last one?

Flags: needinfo?(jorgk) → needinfo?(alta88)

Yes, the last one is best, if least descriptive, but at least not possibly inaccurate. I'd guess there's a 50/50 chance you transfer the error to here:
https://searchfox.org/comm-central/source/mail/base/content/multimessageview.js#736

Flags: needinfo?(alta88)

Well, that will be better than a crash. Should we add a try/catch there?

We can do a follow-up patch if the try/catch is desired.

Target Milestone: --- → Thunderbird 71.0

r+ inferred from comment #8.

Assignee: nobody → jorgk
Attachment #9097181 - Flags: review+

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/dd116f5f342a
Add null check in nsMsgDBView::GetThreadContainingMsgHdr() to avoid crash. r=alta88 DONTBUILD

Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.