###!!! ASSERTION: nsSMimeVerificationListener not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file C:/mozilla/mailnews/mime/src/mimecms.cpp, line 273

RESOLVED FIXED

Status

defect
--
minor
RESOLVED FIXED
12 years ago
8 years ago

People

(Reporter: neil, Assigned: Bienvenu)

Tracking

Trunk
x86
Windows XP
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [psm-logic])

Attachments

(2 attachments, 3 obsolete attachments)

Reporter

Description

12 years ago
I was reading a message with signed by a certificate issued by an unknown CA.

Note that for whatever reason I didn't get the assert in a few weeks old build.

NS_DebugBreak_P(unsigned int 1, const char * 0x04539cb8, const char * 0x04539c84, const char * 0x04539c58, int 273) line 350 + 12 bytes
nsSMimeVerificationListener::Release(nsSMimeVerificationListener * const 0x06c4fbc8) line 273 + 110 bytes
nsCOMPtr<nsISMimeVerificationListener>::~nsCOMPtr<nsISMimeVerificationListener>() line 584
nsSMimeVerificationJob::~nsSMimeVerificationJob() line 88 + 50 bytes
nsSMimeVerificationJob::`scalar deleting destructor'(unsigned int 1) + 15 bytes
nsCertVerificationThread::Run() line 154 + 30 bytes
nsPSMBackgroundThread::nsThreadRunner(void * 0x048bb980) line 45
_PR_NativeRunThread(void * 0x04aa5448) line 436 + 13 bytes
pr_root(void * 0x04aa5448) line 122 + 13 bytes
_threadstartex(void * 0x04aa56d0) line 212 + 13 bytes
Reporter

Comment 1

12 years ago
Subsequent assertion:

###!!! ASSERTION: PipUIContext not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file C:/mozilla/security/manager/ssl/src/nsNSSComponent.cpp, line 2378
Confirming. Stumbled on both assertions when switching from an IMAP folder to a newsgroup with new unread messages, and clicking on one unread newsgroup message.

However, strangely, my backtrace on gdb looks different even though it trapped the assertion.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Posted file first backtrace (obsolete) —
Better backtrace (normal and full) for comment #0.
Attachment #322875 - Attachment is obsolete: true
Posted file second backtrace (obsolete) —
Better backtrace (normal and full) for comment #1.
Just received an email from IMAP exchange on Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090210 Lightning/1.0pre Shredder/3.0b2pre:

###!!! ASSERTION: nsSMimeVerificationListener not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /Users/skywalker/comm-central/mailnews/mime/src/mimecms.cpp, line 276
nsGetServiceByCIDWithError::nsGetServiceByCIDWithError(nsID const&, unsigned int*)+0x000035C3 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libmime.dylib +0x00045857]
nsDeque::GetSize() const+0x00000136 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x0000C088]
nsDeque::GetSize() const+0x000001ED [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x0000C13F]
nsAutoLock::~nsAutoLock()+0x000026B9 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x0000B943]
nsAutoMonitor::~nsAutoMonitor()+0x00003479 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x00008EAF]
PR_Select+0x00000491 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/libnspr4.dylib +0x000292CD]
_pthread_start+0x00000141 [/usr/lib/libSystem.B.dylib +0x00032095]
thread_start+0x00000022 [/usr/lib/libSystem.B.dylib +0x00031F52]

Obsoleting my stacktraces.
Attachment #344050 - Attachment is obsolete: true
Attachment #344051 - Attachment is obsolete: true
This comes immediately after the previous stacktrace in comment #6:


###!!! ASSERTION: PipUIContext not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /Users/skywalker/comm-central/mozilla/security/manager/ssl/src/nsNSSComponent.cpp, line 2733
nsAutoLock::unlock()+0x00005AA3 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x00017B8D]
nsGetInterface::nsGetInterface(nsISupports*, unsigned int*)+0x00000380 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x00011EEA]
nsGetServiceByCID::nsGetServiceByCID(nsID const&)+0x00006CE5 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x00055721]
nsGetServiceByCID::nsGetServiceByCID(nsID const&)+0x000068F9 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x00055335]
nsDeque::GetSize() const+0x000000F2 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x0000C044]
nsDeque::GetSize() const+0x000001FB [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x0000C14D]
nsAutoLock::~nsAutoLock()+0x000026B9 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x0000B943]
nsAutoMonitor::~nsAutoMonitor()+0x00003479 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/components/libpipnss.dylib +0x00008EAF]
PR_Select+0x00000491 [/Users/skywalker/objdir-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/libnspr4.dylib +0x000292CD]
_pthread_start+0x00000141 [/usr/lib/libSystem.B.dylib +0x00032095]
thread_start+0x00000022 [/usr/lib/libSystem.B.dylib +0x00031F52]
PipUIContext doesn't contain any state (no member variables).

I guess it's OK to change file nsNSSComponent.cpp and
  NS_IMPL_ISUPPORTS1(PipUIContext, nsIInterfaceRequestor)
to use
  NS_IMPL_THREADSAFE_ISUPPORTS1

Updated

10 years ago
Component: Security: S/MIME → Security: S/MIME
Product: Core → MailNews Core

Updated

9 years ago
Assignee: kaie → nobody
Whiteboard: [psm-logic]
Assignee

Comment 9

9 years ago
Assignee: nobody → bienvenu
Status: NEW → ASSIGNED
Attachment #482940 - Flags: review?(kaie)
Assignee

Comment 10

9 years ago
I had to change both these files to get rid of the assertions when I click on a signed message.
Attachment #482941 - Flags: review?(kaie)
Assignee

Comment 11

9 years ago
pinging for review...
Comment on attachment 482941 [details] [diff] [review]
moz-central part of fix

This is OK, per comment 8
r=kaie
Attachment #482941 - Flags: review?(kaie) → review+
Comment on attachment 482940 [details] [diff] [review]
comm central part of fix - checked in w/ suggested comment.

I was reading code to ensure this is a safe change.
While I did, I wrote down my thinking, and I propose to add it as a comment.

If you agree with the following comment, please add it prior to checkin,
and r=kaie


 class nsSMimeVerificationListener
 {
   ...
   protected:
   // It is safe to declare this implementation as thread safe,
   // despite not using a lock to protect the members.
   // Because of the way the object will be used, we don't expect a race.
   // After construction, the object is passed to another thread,
   // but will no longer be accessed on the original thread.
   // The other thread is unable to access/modify self's data members.
   // When the other thread is finished, it will call into the "Notify"
   // callback. Self's members will be accessed on the other thread,
   // but this is fine, because there is no race with the original thread.
   // Race-protection for XPCOM reference counting is sufficient.

   nsCOMPtr<nsIMsgSMIMEHeaderSink> mHeaderSink;
   ...
 };
Attachment #482940 - Flags: review?(kaie) → review+
Assignee

Comment 14

9 years ago
Thx very much, Kaie. I'm fine with that comment. I'll attach a patch with it in a minute.

Is security/manager/ssl/src under the same checkin rules as the rest of core, i.e., I'd need to get 2.0 approval?
Assignee

Comment 15

9 years ago
Comment on attachment 482941 [details] [diff] [review]
moz-central part of fix

This quiets a thread-safety assertion in Thunderbird whenever a signed message is read, and the object is perfectly threadsafe because it has no member vars (it just inherits from nsIInterfaceRequestor).
Attachment #482941 - Flags: approval2.0?
(In reply to comment #14)
> Thx very much, Kaie. I'm fine with that comment. I'll attach a patch with it in
> a minute.
> 
> Is security/manager/ssl/src under the same checkin rules as the rest of core,
> i.e., I'd need to get 2.0 approval?

yes.
security/manager is part of the core mozilla platform and has the same rules.
Assignee

Updated

9 years ago
Attachment #482940 - Attachment description: comm central part of fix → comm central part of fix - checked in w/ suggested comment.
Please attach a final patch with the correct code comment as suggested in the bug.
Attachment #482941 - Flags: approval2.0? → approval2.0-
Assignee

Comment 18

9 years ago
(In reply to comment #17)
> Please attach a final patch with the correct code comment as suggested in the
> bug.

the suggested comment was for the comm-central patch, not the moz-central patch. re-requesting approval.
Assignee

Updated

9 years ago
Attachment #482941 - Flags: approval2.0- → approval2.0?

Updated

8 years ago
Attachment #482941 - Flags: approval2.0? → approval2.0+
Comment on attachment 482941 [details] [diff] [review]
moz-central part of fix

This was approved on January 20th but not landed. Removing approval as we've closed for the last beta. Please renominate for approval with risk v. reward statement if you think it should go into RC.
Attachment #482941 - Flags: approval2.0+
This is likely to go in after RC but setting checkin-needed in case it slips off our radar..
Keywords: checkin-needed
Depends on: post2.0
Keywords: checkin-needed
Assignee

Updated

8 years ago
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/e901d6de0cc8
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.