Last Comment Bug 882424 - crash in SignedStatusRunnable::SignedStatusRunnable @ nsXPCWrappedJS::AddRef
: crash in SignedStatusRunnable::SignedStatusRunnable @ nsXPCWrappedJS::AddRef
Status: RESOLVED FIXED
: crash, regression, topcrash
Product: MailNews Core
Classification: Components
Component: MIME (show other bugs)
: 24
: All All
: -- critical (vote)
: Thunderbird 24.0
Assigned To: Josh Matthews [:jdm] (on vacation until Dec 5)
:
:
Mentors:
Depends on: 885292
Blocks: 770840 773610
  Show dependency treegraph
 
Reported: 2013-06-12 14:16 PDT by Scoobidiver (away)
Modified: 2013-06-25 05:22 PDT (History)
10 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
unaffected
+
fixed


Attachments
Avoid addreffing the SMIME header sink off the main thread. (4.15 KB, patch)
2013-06-13 14:00 PDT, Josh Matthews [:jdm] (on vacation until Dec 5)
no flags Details | Diff | Splinter Review
Avoid addreffing the SMIME header sink off the main thread. (4.71 KB, patch)
2013-06-14 08:14 PDT, Josh Matthews [:jdm] (on vacation until Dec 5)
Pidgeot18: review+
Details | Diff | Splinter Review
Missing icon (22.58 KB, image/png)
2013-06-14 09:34 PDT, Joshua Cranmer [:jcranmer]
no flags Details
Avoid addreffing the SMIME header sink off the main thread. (4.76 KB, patch)
2013-06-19 07:06 PDT, Josh Matthews [:jdm] (on vacation until Dec 5)
no flags Details | Diff | Splinter Review

Description Scoobidiver (away) 2013-06-12 14:16:09 PDT
Same signature and regression as bug 882200/bug 882125 but for MIME.

Frame 	Module 	Signature 	Source
0 	xul.dll 	nsXPCWrappedJS::AddRef 	js/xpconnect/src/XPCWrappedJS.cpp:158
1 	xul.dll 	nsXPTCStubBase::AddRef 	xpcom/reflect/xptcall/src/xptcall.cpp:30
2 	xul.dll 	SignedStatusRunnable::SignedStatusRunnable 	mailnews/mime/src/mimecms.cpp:273
3 	xul.dll 	ProxySignedStatus 	mailnews/mime/src/mimecms.cpp:288
4 	xul.dll 	nsSMimeVerificationListener::Notify 	mailnews/mime/src/mimecms.cpp:349
5 	xul.dll 	nsSMimeVerificationJob::Run 	security/manager/ssl/src/nsCertVerificationThread.cpp:85
6 	xul.dll 	nsCertVerificationThread::Run 	security/manager/ssl/src/nsCertVerificationThread.cpp:143
7 	nspr4.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:397
8 	nspr4.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:90
9 	msvcr100.dll 	_callthreadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:314
10 	msvcr100.dll 	_threadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:292
11 	kernel32.dll 	kernel32.dll@0x6d084 	
12 	ntdll.dll 	ntdll.dll@0x60afd 	
13 	ntdll.dll 	ntdll.dll@0x60acb

More reports at:
https://crash-stats.mozilla.com/report/list?product=Thunderbird&signature=nsXPCWrappedJS%3A%3AAddRef%28%29
Comment 1 Joshua Cranmer [:jcranmer] 2013-06-12 15:04:07 PDT
nsSMimeVerificationListener::mHeaderSink needs to be an nsMainThreadPtrHandle, as does SignedStatusRunnable::m_sink.
Comment 2 Jonathan Protzenko [:protz] 2013-06-13 11:57:46 PDT
Just for the record, this make me unable to read of one my emails, as Thunderbird crashes when trying to view it (100% reproducible).
Comment 3 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-06-13 14:00:36 PDT
Created attachment 762301 [details] [diff] [review]
Avoid addreffing the SMIME header sink off the main thread.

Bienvenu looks like the person who's reviewed substantial work here before, but I'm happy for anyone else to take it.
Comment 4 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-06-13 14:01:20 PDT
Comment on attachment 762301 [details] [diff] [review]
Avoid addreffing the SMIME header sink off the main thread.

Jonathan, if you wanted to try this out, that would be swell.
Comment 5 Jonathan Protzenko [:protz] 2013-06-14 00:39:05 PDT
Sure. Building right now!
Comment 6 Jonathan Protzenko [:protz] 2013-06-14 03:47:01 PDT
It partly fixes my problem. Using my addon (Thunderbird Conversations), I can view the message. Viewing it without the addon does trigger the crash below, however. I suspect this is because Conversations skips most of the S/MIME machinery (we don't display information and don't poke at the S/MIME functions to figure out whether the email is signed, or not...).

(gdb) bt
#0  0x00007f53b2c3645d in nanosleep () at ../sysdeps/unix/syscall-template.S:81
#1  0x00007f53b2c36301 in __sleep (seconds=0) at ../sysdeps/unix/sysv/linux/sleep.c:137
#2  0x00007f53ada28302 in ah_crap_handler (signum=11) at /home/jonathan/Code/comm-central/mozilla/toolkit/xre/nsSigHandlers.cpp:88
#3  0x00007f53ada2f252 in nsProfileLock::FatalSignalHandler (signo=11, info=<optimized out>, context=<optimized out>)
    at /home/jonathan/Code/comm-central/obj-x86_64-unknown-linux-gnu/mozilla/toolkit/profile/nsProfileLock.cpp:190
#4  <signal handler called>
#5  SignedStatusRunnable::Run (this=0x7f536f46f3a0) at /home/jonathan/Code/comm-central/mailnews/mime/src/mimecms.cpp:279
#6  0x00007f53aef48ab8 in nsThreadSyncDispatch::Run (this=0x7f536f6e3c70) at /home/jonathan/Code/comm-central/mozilla/xpcom/threads/nsThread.cpp:773
#7  0x00007f53aef49180 in nsThread::ProcessNextEvent (this=0x7f53b2a52d50, mayWait=false, result=0x7fff515f841f) at /home/jonathan/Code/comm-central/mozilla/xpcom/threads/nsThread.cpp:626
#8  0x00007f53aef075fa in NS_ProcessNextEvent (thread=<optimized out>, mayWait=mayWait@entry=false)
    at /home/jonathan/Code/comm-central/obj-x86_64-unknown-linux-gnu/mozilla/xpcom/build/nsThreadUtils.cpp:238
#9  0x00007f53aebbd8f9 in mozilla::ipc::MessagePump::Run (this=0x7f53b2afdc40, aDelegate=0x7f53a160a240) at /home/jonathan/Code/comm-central/mozilla/ipc/glue/MessagePump.cpp:82
#10 0x00007f53aef797aa in MessageLoop::RunInternal (this=this@entry=0x7f53a160a240) at /home/jonathan/Code/comm-central/mozilla/ipc/chromium/src/base/message_loop.cc:219
#11 0x00007f53aef797d2 in RunHandler (this=0x7f53a160a240) at /home/jonathan/Code/comm-central/mozilla/ipc/chromium/src/base/message_loop.cc:212
#12 MessageLoop::Run (this=0x7f53a160a240) at /home/jonathan/Code/comm-central/mozilla/ipc/chromium/src/base/message_loop.cc:186
#13 0x00007f53ae83670f in nsBaseAppShell::Run (this=0x7f53a01c4160) at /home/jonathan/Code/comm-central/mozilla/widget/xpwidgets/nsBaseAppShell.cpp:163
#14 0x00007f53ae686683 in nsAppStartup::Run (this=0x7f539d471470) at /home/jonathan/Code/comm-central/mozilla/toolkit/components/startup/nsAppStartup.cpp:269
#15 0x00007f53ada23cf9 in XREMain::XRE_mainRun (this=this@entry=0x7fff515f86d8) at /home/jonathan/Code/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:3856
#16 0x00007f53ada266d0 in XREMain::XRE_main (this=this@entry=0x7fff515f86d8, argc=argc@entry=1, argv=argv@entry=0x7fff515f9ac8, aAppData=aAppData@entry=0x7f53b2a25800)
    at /home/jonathan/Code/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:3924
#17 0x00007f53ada26930 in XRE_main (argc=1, argv=0x7fff515f9ac8, aAppData=0x7f53b2a25800, aFlags=<optimized out>) at /home/jonathan/Code/comm-central/mozilla/toolkit/xre/nsAppRunner.cpp:4126
#18 0x00000000004033a0 in do_main (argv=0x7fff515f9ac8, argc=1, exePath=0x7fff515f89b0 "/home/jonathan/Code/comm-central/obj-x86_64-unknown-linux-gnu/mozilla/dist/bin/")
    at /home/jonathan/Code/comm-central/mail/app/nsMailApp.cpp:111
#19 main (argc=1, argv=0x7fff515f9ac8) at /home/jonathan/Code/comm-central/mail/app/nsMailApp.cpp:200
Comment 7 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-06-14 07:53:07 PDT
Is m_sink a valid object when that crash occurs?
Comment 8 Joshua Cranmer [:jcranmer] 2013-06-14 08:10:59 PDT
(In reply to Jonathan Protzenko [:protz] from comment #6)
> It partly fixes my problem. Using my addon (Thunderbird Conversations), I
> can view the message. Viewing it without the addon does trigger the crash
> below, however. I suspect this is because Conversations skips most of the
> S/MIME machinery (we don't display information and don't poke at the S/MIME
> functions to figure out whether the email is signed, or not...).

I get roughly this same stack just when I loaded a signed email (but not an encrypted email).

(In reply to Josh Matthews [:jdm] from comment #7)
> Is m_sink a valid object when that crash occurs?

(gdb) p m_sink
$1 = {mPtr = {mRawPtr = 0x7f58c06c8960}}
(gdb) p *m_sink.mPtr.mRawPtr
$5 = (nsMainThreadPtrHolder<nsIMsgSMIMEHeaderSink>) {
  _vptr.nsMainThreadPtrHolder = 0x7f58de354c30 <vtable for nsMainThreadPtrHolder<nsIMsgSMIMEHeaderSink>+16>, mRefCnt = {mValue = 2}, mRawPtr = 0x0, 
  mStrict = true}
Comment 9 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-06-14 08:14:59 PDT
Created attachment 762708 [details] [diff] [review]
Avoid addreffing the SMIME header sink off the main thread.

Try this patch instead.
Comment 10 Jonathan Protzenko [:protz] 2013-06-14 08:41:54 PDT
This fixes it for me. Thanks!
Comment 11 Joshua Cranmer [:jcranmer] 2013-06-14 08:43:15 PDT
The patch stops crashing, but the notification disappears.
Comment 12 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-06-14 09:22:49 PDT
Which notification is that?
Comment 13 Joshua Cranmer [:jcranmer] 2013-06-14 09:34:55 PDT
Created attachment 762738 [details]
Missing icon

The icon of the envelope in the upper-right corner appears to be missing.
Comment 14 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-06-14 09:53:26 PDT
Could you try a build without this patch and with bug 770840 rolled back? I don't really see how the changes here could have affected anything else; the only semantic difference chould be that the sink object is always released on the main thread in all cases now, potentially a bit later than it used to be.
Comment 15 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-06-14 14:41:40 PDT
(In reply to Josh Matthews [:jdm] from comment #14)
> Could you try a build without this patch and with bug 770840 rolled back?

I'm seeing this crash multiple times a day, usually when trying to load a message in the preview pane while a lot of messages are downloading in the background. Josh, would it be possible for you to generate a try-server build for me to test?
Comment 16 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-06-14 14:48:36 PDT
Actually, I have an email in my inbox which reproduces this quite reliably now. I'd be happy to do some testing if it will help resolve this issue.
Comment 17 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-06-14 16:56:22 PDT
(In reply to Anthony Hughes, Mozilla QA (:ashughes) from comment #16)
> Actually, I have an email in my inbox which reproduces this quite reliably
> now. I'd be happy to do some testing if it will help resolve this issue.

Anthony, it appears that this patch resolves the crashes. I'm more interested in exploring comment 11, which should just require testing a Thunderbird nightly from before bug 770840 landed.
Comment 18 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-06-17 11:53:29 PDT
I do not crash with Thunderbird 24.0a1 2013-06-10 which should be from before bug 770840 landed.
Comment 19 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-06-17 15:29:20 PDT
Please note that I'm not asking about the crashing behaviour. It is comment 11 that interests me specifically.
Comment 20 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-06-17 17:18:09 PDT
(In reply to Josh Matthews [:jdm] from comment #19)
> Please note that I'm not asking about the crashing behaviour. It is comment
> 11 that interests me specifically.

I'll have to defer to @jcranmer on that. I can't ever recall seeing that "envelope" icon in Thunderbird but I use Linux so maybe it's different. 

FWIW, the crash is far more concerning since Thunderbird Daily is completely unusable with this bug. I think comment 11 (ie. envelope icon appearance) should be spun off into a dependent bug; the scope of this bug should be limited to fixing this crash. That's just my personal opinion though.
Comment 21 Wayne Mery (:wsmwk, NI for questions) 2013-06-17 18:31:21 PDT
Indeed, a significant stability problem. #1 crash by a long shot - https://crash-stats.mozilla.com/topcrasher/byversion/Thunderbird/24.0a1/3/browser

I've randomly crashed 6 times in 5 days.
Comment 22 Hiroyuki Ikezoe (:hiro) 2013-06-18 21:37:15 PDT
I was hit by this crash today. After applied attachment 762708 [details] [diff] [review], this crash has gone but I got another ASSERTION:

###!!! ASSERTION: Can't dereference nsMainThreadPtrHolder off main thread: 'Error', file ../../../mozilla/dist/include/nsProxyRelease.h, line 137
Comment 23 Hiroyuki Ikezoe (:hiro) 2013-06-18 21:42:10 PDT
backtrace:

#0  0x00007fce19b7883d in nanosleep () at ../sysdeps/unix/syscall-template.S:82
#1  0x00007fce19b786dc in __sleep (seconds=0) at ../sysdeps/unix/sysv/linux/sleep.c:138
#2  0x00007fce144f447d in ah_crap_handler (signum=11) at /home/zoe/hg/comm-central/mozilla/toolkit/xre/nsSigHandlers.cpp:88
#3  0x00007fce14500198 in nsProfileLock::FatalSignalHandler (signo=11, info=0x7fcddfffe970, context=0x7fcddfffe840)
    at /home/zoe/hg/comm-central/objdir-thunderbird/mozilla/toolkit/profile/nsProfileLock.cpp:190
#4  <signal handler called>
#5  0x00007fce161e51da in nsMainThreadPtrHolder<nsIMsgSMIMEHeaderSink>::get (this=0x2f14d80) at ../../../mozilla/dist/include/nsProxyRelease.h:138
#6  0x00007fce161e4e54 in nsMainThreadPtrHandle<nsIMsgSMIMEHeaderSink>::get (this=0x562e2e8) at ../../../mozilla/dist/include/nsProxyRelease.h:191
#7  0x00007fce161e3786 in nsSMimeVerificationListener::Notify (this=0x562e2d0, aVerifiedMessage=0x171a318, aVerificationResultCode=NS_OK) at /home/zoe/hg/comm-central/mailnews/mime/src/mimecms.cpp:312
#8  0x00007fce15a72126 in nsSMimeVerificationJob::Run (this=0x1d0c070) at /home/zoe/hg/comm-central/mozilla/security/manager/ssl/src/nsCertVerificationThread.cpp:85
#9  0x00007fce15a724ae in nsCertVerificationThread::Run (this=0x13673b0) at /home/zoe/hg/comm-central/mozilla/security/manager/ssl/src/nsCertVerificationThread.cpp:143
#10 0x00007fce15acc2f5 in nsPSMBackgroundThread::nsThreadRunner (arg=0x13673b0) at /home/zoe/hg/comm-central/mozilla/security/manager/ssl/src/nsPSMBackgroundThread.cpp:14
#11 0x00007fce1aa6fdf1 in _pt_root (arg=0x1367660) at /home/zoe/hg/comm-central/mozilla/nsprpub/pr/src/pthreads/ptthread.c:204
#12 0x00007fce1a67fe9a in start_thread (arg=0x7fcddffff700) at pthread_create.c:308
#13 0x00007fce19baccbd in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
Comment 24 Hiroyuki Ikezoe (:hiro) 2013-06-18 22:09:23 PDT
Comment on attachment 762708 [details] [diff] [review]
Avoid addreffing the SMIME header sink off the main thread.

> NS_IMETHODIMP nsSMimeVerificationListener::Notify(nsICMSMessage2 *aVerifiedMessage,
>                                                   nsresult aVerificationResultCode)
> {
>   // Only continue if we have a valid pointer to the UI
>-  NS_ENSURE_TRUE(mHeaderSink, NS_OK);
>+  NS_ENSURE_TRUE(mHeaderSink.get(), NS_OK);

Seems to be stable without this modification.
Comment 25 Joshua Cranmer [:jcranmer] 2013-06-19 06:26:58 PDT
Comment on attachment 762708 [details] [diff] [review]
Avoid addreffing the SMIME header sink off the main thread.

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

I'm concerned that the icon for signed status verification isn't showing up for me on Linux, but I can't see how this patch would be causing it, and this is a major crasher otherwise. It's best to land this and see if others report the loss of signed status verification.
Comment 26 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-06-19 07:06:19 PDT
Created attachment 764734 [details] [diff] [review]
Avoid addreffing the SMIME header sink off the main thread.

This should address the assertion in comment 22 (thanks!)
Comment 27 Josh Matthews [:jdm] (on vacation until Dec 5) 2013-06-19 07:09:15 PDT
https://hg.mozilla.org/comm-central/rev/7a6a3257525d

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