crash SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol* ... typically working with folder subscribe

VERIFIED FIXED in Thunderbird 19.0

Status

MailNews Core
Networking: IMAP
--
critical
VERIFIED FIXED
5 years ago
3 years ago

People

(Reporter: wsmwk, Assigned: m_kato)

Tracking

(4 keywords)

Trunk
Thunderbird 19.0
x86
Windows NT
crash, regression, testcase-wanted, topcrash

Thunderbird Tracking Flags

(thunderbird12-, thunderbird13-, thunderbird17 fixed, thunderbird18 fixed)

Details

(Whiteboard: [dataloss?][regression:tb10], crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-251126bf-1124-43b5-bd29-656872120202 .
============================================================= 

#14 crash currently for version 10
regression of Bug 681188 - Switch from PRBool to bool ?

0	xul.dll	xul.dll@0x8469f1	
1	xul.dll	`anonymous namespace'::SyncRunnable5<nsIImapMailFolderSink,nsIImapProtocol*,nsIMsgMailNewsUrl*,bool,bool,unsigned int>::Run	mailnews/imap/src/nsSyncRunnableHelpers.cpp:282
2	xul.dll	nsThread::ProcessNextEvent	xpcom/threads/nsThread.cpp:631
3	xul.dll	NS_ProcessNextEvent_P	objdir-tb/mozilla/xpcom/build/nsThreadUtils.cpp:245
4	xul.dll	mozilla::ipc::MessagePump::Run	ipc/glue/MessagePump.cpp:134
5	xul.dll	MessageLoop::RunInternal	ipc/chromium/src/base/message_loop.cc:208
6	xul.dll	MessageLoop::RunHandler	ipc/chromium/src/base/message_loop.cc:201
7	xul.dll	MessageLoop::Run	ipc/chromium/src/base/message_loop.cc:175
8	xul.dll	nsBaseAppShell::Run	widget/src/xpwidgets/nsBaseAppShell.cpp:189
9	xul.dll	nsAppShell::Run	widget/src/windows/nsAppShell.cpp:257
10	xul.dll	nsAppStartup::Run	toolkit/components/startup/nsAppStartup.cpp:228
As a topcrash regression, we should make an effort to resolve this for a near term release.

most comments cite doing imap folder subscriptions.
bp-62b0a6a5-2a42-4787-80c0-5be9f2120221 cites dataloss
"trying to subscribe to folders which Thuinderburd keeps losing in the subscription list total pain in the butt and a game breaker as far as I'm concerned"

Mac and linux @ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run
Crash Signature: [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] → [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip;
tracking-thunderbird12: --- → ?
tracking-thunderbird13: --- → ?
Summary: crash xul → crash nsIImapMailFolderSink, nsIImapProtocol*
Whiteboard: [dataloss?]

Comment 2

5 years ago
I haven't been able to reproduce this. A protocol log might be helpful.
not a straight up regression of Bug 681188 - Switch from PRBool to bool - given that it landed in v10?
Crash Signature: [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip; → [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip;

Comment 4

5 years ago
I suspect this would have come from switching to sync runnable, i.e., getting rid of xpcom proxies in Bug 675407
(Reporter)

Updated

5 years ago
Blocks: 675407
Summary: crash nsIImapMailFolderSink, nsIImapProtocol* → crash SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol* ... typically working with folder subscribe
Keywords: testcase-wanted
re: testcase

several crash reporters have been contacted. however, we should not count on  finding a testcase from this population of users - historically it is difficult to  impossible to get a cooperating victim
(Reporter)

Updated

5 years ago
Crash Signature: [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip; → [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip;
Keywords: topcrash → topcrash+
doesn't seem to be a persistent crash - only 1 in 4 people replied to me, and of those only 1 in 5 can reproduce.  

ludo you have just gotten an email with screen shot for that person who can reproduce
Crash Signature: [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip; → [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip;
Keywords: topcrash+ → topcrash
tracking-thunderbird12: ? → -
tracking-thunderbird13: ? → -

Comment 7

5 years ago
I'm pretty sure this is caused by a null folder sink. The sync runnable changes just changed the stack signature, but I couldn't be surprised if the crash was around before (hard to know what the old stack would have looked like, since it's on the other side of the event loop now). It's very hard to see where subscribe UI would end up with a null folder sink that we're not checking for.

Comment 8

5 years ago
Oh, I should say, pretty sure this is a call to SetUrlState

Comment 9

5 years ago
from one of the crashes, this is the call site:

http://hg.mozilla.org/releases/comm-release/annotate/6ccee99d387c/mailnews/imap/src/nsImapProtocol.cpp#l1602

Since we're checking for m_imapMailFolderSink is null before calling this, we might have some sort of race condition, if I'm understanding the crash info correctly.
Flags: in-testsuite+
also 
@0x0 | nsImapMailFolder::SetUrlState(nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int) 
and
nsImapMailFolder::SetUrlState(nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int)
Crash Signature: [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip; → [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip;

Updated

5 years ago
Duplicate of this bug: 763888
Crash Signature: [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip; → [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip;
bienvenu, 

bp-40cf7199-6e99-4ac0-ab7f-3f81c2120717 is a user who is crashing to beat the band - many times a day. Mostly this signature, but also a smattering of others. like ...

nsRefPtr<nsCanvasPatternAzure>::~nsRefPtr<nsCanvasPatternAzure>() | nsMsgFilterAfterTheFact::~nsMsgFilterAfterTheFact() 
bp-8547793c-5222-4166-b3c1-481402120717

mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | ToNewUnicode(nsACString_internal const&) 
bp-5b2f0c03-4543-48de-a895-61fb22120717

Collect 
bp-075b305d-75c3-4637-8b5c-6ad582120718
#23 crash TB14

TB14 signature xul.dll@0x85be1d | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()
Crash Signature: [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip; → [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip;

Comment 14

5 years ago
It looks like we just had somebody report this crash in Launchpad too:

https://launchpad.net/bugs/1052306

According to the bug report data, the reporter has just submitted https://crash-stats.mozilla.com/report/index/bp-2db3eea4-c46d-4d64-8b6e-e47f92120918, and they say that the crash happens on a specific mail
Created attachment 664374 [details] [diff] [review]
possible fix
Assignee: nobody → m_kato
Comment on attachment 664374 [details] [diff] [review]
possible fix

mReceiver seems to be null.  We should not pass null to constructor of ImapMailFolderSinkProxy.
Attachment #664374 - Flags: review?(kent)

Comment 17

5 years ago
Comment on attachment 664374 [details] [diff] [review]
possible fix

I'm going to pass this review on to :bienvenu since he has been intimately involved in the discussions so far. If you don't get a review in the next week or so, feel free to pass it back to me (or irving or standard8) and we'll figure something out.
Attachment #664374 - Flags: review?(kent) → review?(mozilla)

Updated

5 years ago
(Reporter)

Updated

5 years ago
Flags: in-testsuite+
Whiteboard: [dataloss?] → [dataloss?][regression:tb10]
Comment on attachment 664374 [details] [diff] [review]
possible fix

could you review this instead of bienvenu?
Attachment #664374 - Flags: review?(mozilla) → review?(mbanner)
(Assignee)

Updated

5 years ago
Crash Signature: [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip; → [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip;
Comment on attachment 664374 [details] [diff] [review]
possible fix

This is probably fine, but Irving is a bit closer to the imap code than I am at the moment.
Attachment #664374 - Flags: review?(mbanner) → review?(irving)
Comment on attachment 664374 [details] [diff] [review]
possible fix

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

Looks OK to me; there's a lot of code that changes behaviour if m_imapMailFolderSink is null so we'll need to keep an eye on trunk in case this moves the crash somewhere else.
Attachment #664374 - Flags: review?(irving) → review+
https://hg.mozilla.org/comm-central/rev/2a62a0c386a8
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Comment on attachment 664374 [details] [diff] [review]
possible fix

[Approval Request Comment]
Regression caused by (bug #): bug 675407
User impact if declined: crash. Actually, this is top 17 crash on 16.0.1.
Testing completed (on c-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): low.  This adds nullptr check. also, m_imapMailFolderSink has already nullptr check.
Attachment #664374 - Flags: approval-comm-beta?
Attachment #664374 - Flags: approval-comm-aurora?
Attachment #664374 - Flags: approval-comm-beta?
Attachment #664374 - Flags: approval-comm-beta+
Attachment #664374 - Flags: approval-comm-aurora?
Attachment #664374 - Flags: approval-comm-aurora+
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/5629f96d8cd4
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/3cb3fc93b072
status-thunderbird17: --- → fixed
status-thunderbird18: --- → fixed
Tb15.0.1 signature xul.dll@0x8c0cdb | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()
Crash Signature: [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip; → [@ xul.dll@0x8469f1 | `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run()] [@ SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool uns&hellip;
Target Milestone: --- → Thunderbird 19.0
crash stats shows most crashing has stopped.
less than a dozen crashes in past weeks for 17.0 for `anonymous namespace''::SyncRunnable5<nsIImapMailFolderSink, nsIImapProtocol*, nsIMsgMailNewsUrl*, bool, bool, unsigned int>::Run() 
bp-a7116160-4238-440b-ba41-4f3952130129 17.0.2
bp-633908f4-4d37-414c-bddd-953cc2130105 17.0
Status: RESOLVED → VERIFIED
(Reporter)

Updated

3 years ago
See Also: → bug 825513
You need to log in before you can comment on or make changes to this bug.