Last Comment Bug 609758 - crash [@ nsMsgSearchSession::GetRunningAdapter(nsIMsgSearchAdapter**)]
: crash [@ nsMsgSearchSession::GetRunningAdapter(nsIMsgSearchAdapter**)]
Status: RESOLVED FIXED
: crash
Product: MailNews Core
Classification: Components
Component: Search (show other bugs)
: Trunk
: x86 All
: -- critical (vote)
: Thunderbird 15.0
Assigned To: David Lechner (:dlech)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-11-04 15:26 PDT by Wayne Mery (:wsmwk, NI for questions)
Modified: 2012-05-01 16:57 PDT (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
add check for existance of scope->m_adapter (1.08 KB, patch)
2012-04-29 22:13 PDT, David Lechner (:dlech)
mozilla: review-
Details | Diff | Review
made change suggested in comment 4 (1.15 KB, patch)
2012-04-30 16:09 PDT, David Lechner (:dlech)
mozilla: review+
Details | Diff | Review

Description Wayne Mery (:wsmwk, NI for questions) 2010-11-04 15:26:26 PDT
crash [@ nsMsgSearchSession::GetRunningAdapter(nsIMsgSearchAdapter**)]
almost non-existent
interesting: all OS

2b548a85-9e82-46fc-a22d-7b91c2101007 (thomas)
0	thunderbird-bin	nsMsgSearchSession::GetRunningAdapter	mailnews/base/search/src/nsMsgSearchSession.cpp:566
1	thunderbird-bin	nsMsgSearchSession::OnStopRunningUrl	mailnews/base/search/src/nsMsgSearchSession.cpp:374
2	thunderbird-bin	nsMsgMailNewsUrl::SetUrlState	mailnews/base/util/nsMsgMailNewsUrl.cpp:135
3	thunderbird-bin	nsImapMailFolder::SetUrlState	mailnews/imap/src/nsImapMailFolder.cpp:6603
4	libxpcom_core.so	libxpcom_core.so@0x63f86	
5	libxpcom_core.so	nsProxyObjectCallInfo::Run	xpcom/proxy/src/nsProxyEvent.cpp:181
6	libxpcom_core.so	nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:521 


bp-a812ff1a-b0de-4cc5-ba2e-ad52d2100901 v3.1.2
0	thunderbird.exe	nsMsgSearchSession::GetRunningAdapter	mailnews/base/search/src/nsMsgSearchSession.cpp:584
1	thunderbird.exe	nsImapMailFolder::NotifySearchHit	mailnews/imap/src/nsImapMailFolder.cpp:5574
2	xpcom_core.dll	NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102 


bp-f34641cc-9c0d-4563-b4b0-ca93c2101104 v3.1.7pre
0	thunderbird.exe	nsMsgSearchSession::GetRunningAdapter	mailnews/base/search/src/nsMsgSearchSession.cpp:584
1	thunderbird.exe	nsMsgSearchSession::OnStopRunningUrl	mailnews/base/search/src/nsMsgSearchSession.cpp:392
2	thunderbird.exe	nsMsgMailNewsUrl::SetUrlState	mailnews/base/util/nsMsgMailNewsUrl.cpp:136
3	thunderbird.exe	nsImapMailFolder::SetUrlState	mailnews/imap/src/nsImapMailFolder.cpp:6643
4	xpcom_core.dll	NS_InvokeByIndex_P	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:102
Comment 1 Wayne Mery (:wsmwk, NI for questions) 2011-08-24 21:15:23 PDT
1 crash per month for 3.1 ending at 3.1.10. 
And none for v5 or v6.
Comment 2 David Lechner (:dlech) 2012-04-29 22:13:40 PDT
Created attachment 619491 [details] [diff] [review]
add check for existance of scope->m_adapter

I caused a crash here while working on an extension. This particular case involved an IMAP server. It is possible to create a search without an adapter, which I did. The adapter is not created until you run the search. Then, I manually sent a search command to the server using the nsIMsgImapMailFolder.issueCommandOnMsgs function.

The crash was caused by scope->m_adapter being a null pointer. I added a check for this so that when it happens, it fails gracefully instead of crashing.
Comment 3 Wayne Mery (:wsmwk, NI for questions) 2012-04-30 05:26:17 PDT
David, can you request review for your patch?

(fwiw, no crashes found for past 6 weeks. but reopening to accept patch)
Comment 4 David :Bienvenu 2012-04-30 10:59:51 PDT
Comment on attachment 619491 [details] [diff] [review]
add check for existance of scope->m_adapter

thx for the patch. You should be able to use NS_IF_ADDREF(*aSearchAdapter = scope->m_adapter); If that works, I'd give an r+ for that.
Comment 5 David Lechner (:dlech) 2012-04-30 16:09:03 PDT
Created attachment 619737 [details] [diff] [review]
made change suggested in comment 4

Changed patch to use NS_IF_ADDREF. Also moved the *aSearchAdapter = nsnull; line to ensure that *aSearchAdapter is assigned a value in all cases.
Comment 6 David Lechner (:dlech) 2012-04-30 16:11:59 PDT
Forgot to mention that I tested it with my broken plugin to verify that the revised patch also fixes the crash.
Comment 7 David :Bienvenu 2012-04-30 17:00:24 PDT
Comment on attachment 619737 [details] [diff] [review]
made change suggested in comment 4

looks ok, thx.
Comment 8 David :Bienvenu 2012-04-30 20:58:54 PDT
Comment on attachment 619737 [details] [diff] [review]
made change suggested in comment 4

you don't need super review for a patch that doesn't change an interface...
Comment 9 Ryan VanderMeulen [:RyanVM] 2012-05-01 16:57:21 PDT
Thanks for the patch!

http://hg.mozilla.org/comm-central/rev/cd7bbb603424

Note You need to log in before you can comment on or make changes to this bug.