330273 - IMAP releases non-threadsafe objects off-thread

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Boris Zbarsky [:bzbarsky]]

STEPS TO REPRODUCE:

1)  Set up an IMAP account on a server that requires SSL but do NOT set IMAP to
    use SSL (this is what the account wizard produces).
2)  Open mailnews.
3)  Click "ok" on the "couldn't connect" alert.

EXPECTED RESULTS: No assert.

ACTUAL RESULTS:

###!!! ASSERTION: nsWeakReference not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file nsWeakReference.cpp, line 110

which happens at:

#0  Break (
    aMsg=0xb386cc90 "###!!! ASSERTION: nsWeakReference not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file nsWeakReference.cpp, line 110")
    at ../../../mozilla/xpcom/base/nsDebugImpl.cpp:471
#1  0xb7f99bea in NS_DebugBreak_P (aSeverity=1, 
    aStr=0xb7fc70b0 "nsWeakReference not thread-safe", 
    aExpr=0xb7fc707c "_mOwningThread.GetThread() == PR_GetCurrentThread()", 
    aFile=0xb7fc6fe4 "nsWeakReference.cpp", aLine=110)
    at ../../../mozilla/xpcom/base/nsDebugImpl.cpp:354
#2  0xb7f09f6a in nsWeakReference::Release (this=0x884d6c8) at nsWeakReference.cpp:110
#3  0xb3a1c6d1 in nsCOMPtr<nsIWeakReference>::assign_assuming_AddRef (this=0xb3c6ac54, 
    newPtr=0x0) at ../../../dist/include/xpcom/nsCOMPtr.h:568
#4  0xb3a1ce74 in nsCOMPtr<nsIWeakReference>::assign_with_AddRef (this=0xb3c6ac54, 
    rawPtr=0x0) at ../../../dist/include/xpcom/nsCOMPtr.h:1224
#5  0xb3a1ae91 in nsCOMPtr<nsIWeakReference>::operator= (this=0xb3c6ac54, rhs=0x0)
    at ../../../dist/include/xpcom/nsCOMPtr.h:713
#6  0xb39ff2a7 in nsImapProtocol::CloseStreams (this=0xb3c6ab30)
    at ../../../../mozilla/mailnews/imap/src/nsImapProtocol.cpp:975
#7  0xb39ff5d8 in nsImapProtocol::TellThreadToDie (this=0xb3c6ab30, isSafeToClose=0)
    at ../../../../mozilla/mailnews/imap/src/nsImapProtocol.cpp:1039
#8  0xb3a00f71 in nsImapProtocol::ProcessCurrentURL (this=0xb3c6ab30)
    at ../../../../mozilla/mailnews/imap/src/nsImapProtocol.cpp:1474
#9  0xb39ffb6f in nsImapProtocol::ImapThreadMainLoop (this=0xb3c6ab30)
    at ../../../../mozilla/mailnews/imap/src/nsImapProtocol.cpp:114

Comment 1

[David :Bienvenu]

OK, this will happen when we call TellThreadToDie from the imap thread, which we do in several error conditions. We usually call it from the UI thread, which is where we create the weak reference from. I believe the assertion is harmless, if annoying, because the weak reference's owning object is going to get freed right after this routine, and the object it's referring to tends to live for the lifetime of the app. And there are other synchronization mechanisms that prevent the reference from getting manipulated on two threads at the same time. But I'll see what I can do when I'm working on related issues.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

I suspect the patch that was landed as part of bug 410747 should fix this bug too.

Comment 3

[David :Bienvenu]

Attachment: fix for remaining issue

there's an other instance I occasionally see - this should fix it. We proxy the release of the imap url to the ui thread, if it hasn't already been cleared.

Comment 4

[David :Bienvenu]

fix checked in.