1133892 - Thunderbird 31 crash in NS_CycleCollectorSuspect3 and nsXPCWrappedJS::Release() via CopyListener::~CopyListener (nsImapMailCopyState::~nsImapMailCopyState and nsMsgComposeAndSend::~nsMsgComposeAndSend). Release off main thread. [imap]

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Wayne Mery (:wsmwk)]

Probably continuation of crashes previously reported under a different signature.

+++ This bug was initially created as a clone of Bug #1132339 (TB38 only) +++
+++ This bug was initially created as a clone of (FIXED in TB31) Bug #991626 +++

NS_CycleCollectorSuspect3  #10 crash for Thunderbird 31.4.0
nsXPCWrappedJS::Release()  #24 crash for Thunderbird 31.4.0

NS_CycleCollectorSuspect3 
bp-41e5c719-60cc-4eb9-b463-baee52150207  via nsMsgComposeAndSend::~nsMsgComposeAndSend() 
 0 	xul.dll	NS_CycleCollectorSuspect3	xpcom/base/nsCycleCollector.cpp
1 	xul.dll	nsGlobalWindow::Release()	dom/base/nsGlobalWindow.cpp
2 	xul.dll	nsMsgComposeAndSend::~nsMsgComposeAndSend()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/compose/src/nsMsgSend.cpp:323
3 	xul.dll	nsMsgComposeAndSend::`scalar deleting destructor'(unsigned int)	
4 	xul.dll	nsMsgComposeAndSend::Release()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/compose/src/nsMsgSend.cpp:255
5 	xul.dll	CopyListener::~CopyListener()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/compose/src/nsMsgCopy.cpp:49
6 	xul.dll	CopyListener::`scalar deleting destructor'(unsigned int)	
7 	xul.dll	mozilla::net::FileOpenHelper::Release()	netwerk/cache2/CacheFile.cpp  

nsXPCWrappedJS::Release()
bp-a3da1435-0f95-46de-862c-5dcd62150208  via nsMsgProgress::~nsMsgProgress()
bp-af5122c1-2355-4568-b296-b47b32150211  via nsMsgProgress::~nsMsgProgress()
0 	xul.dll	nsXPCWrappedJS::Release()	js/xpconnect/src/XPCWrappedJS.cpp
1 	xul.dll	nsXPTCStubBase::Release()	xpcom/reflect/xptcall/xptcall.cpp
2 	xul.dll	ReleaseObjects	xpcom/glue/nsCOMArray.cpp
3 	xul.dll	nsCOMArray_base::Clear()	xpcom/glue/nsCOMArray.cpp
4 	xul.dll	nsMsgProgress::~nsMsgProgress()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/base/src/nsMsgProgress.cpp:34 
5 	xul.dll	nsMsgProgress::`scalar deleting destructor'(unsigned int)	
6 	xul.dll	nsMsgProgress::Release()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/base/src/nsMsgProgress.cpp:21
7 	xul.dll	nsMsgComposeAndSend::~nsMsgComposeAndSend()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/compose/src/nsMsgSend.cpp:323
8 	xul.dll	nsMsgComposeAndSend::`scalar deleting destructor'(unsigned int)	
9 	xul.dll	nsMsgComposeAndSend::Release()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/compose/src/nsMsgSend.cpp:255
10 	xul.dll	CopyListener::~CopyListener()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/compose/src/nsMsgCopy.cpp:49
11 	xul.dll	CopyListener::`scalar deleting destructor'(unsigned int)	
12 	xul.dll	mozilla::net::FileOpenHelper::Release()	netwerk/cache2/CacheFile.cpp
13 	xul.dll	nsImapMailCopyState::~nsImapMailCopyState()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/imap/src/nsImapMailFolder.cpp:8215
14 	xul.dll	nsImapMailCopyState::`scalar deleting destructor'(unsigned int)	
15 	xul.dll	mozilla::net::WebSocketRequest::Release()	netwerk/base/Dashboard.cpp
16 	xul.dll	nsImapUrl::~nsImapUrl()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/imap/src/nsImapUrl.cpp:79
17 	xul.dll	nsImapUrl::`scalar deleting destructor'(unsigned int)	
18 	xul.dll	nsMsgMailNewsUrl::Release()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/base/util/nsMsgMailNewsUrl.cpp:57
19 	xul.dll	nsSmtpUrl::Release()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/compose/src/nsSmtpUrl.cpp:599
20 	xul.dll	nsMsgProtocol::~nsMsgProtocol()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/base/util/nsMsgProtocol.cpp:83
21 	xul.dll	nsImapProtocol::~nsImapProtocol()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/imap/src/nsImapProtocol.cpp:581
22 	xul.dll	nsImapProtocol::`scalar deleting destructor'(unsigned int)	
23 	xul.dll	nsBaseAppShell::Release()	widget/nsBaseAppShell.cpp
24 	xul.dll	nsImapProtocol::Release()	c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/imap/src/nsImapProtocol.cpp:291 

perhaps unrelated
bp-8cee1d98-3fda-4e77-9e49-1000c2150206  @ js::ShapeTable::search(jsid, bool)

Comment 1

[Javi Rueda :javirid]

I've been seeing a lot this crash for a few days on all 3 Mozilla products: Firefox, Thunderbird and SeaMonkey.

My crash log: https://crash-stats.mozilla.com/report/index/f5211810-a4c7-4c9b-af5d-11c2b2150310

Comment 2

[Wayne Mery (:wsmwk)]

(In reply to Javi Rueda from comment #1)
> I've been seeing a lot this crash for a few days on all 3 Mozilla products:
> Firefox, Thunderbird and SeaMonkey.

True - but they have different root causes. Hence different bug reports. We must chop the roots.

Comment 3

[Wayne Mery (:wsmwk)]

A high perecentage of crash comments cite sending message failed or sent message failed to copy to Sent folder. So it is important that we also treat this topcrash as dataloss

Comment 4

[Wayne Mery (:wsmwk)]

An example is this user with NS_CycleCollectorSuspect3 over a period of months:
bp-506459af-1c1c-4555-be00-e22e02141230
bp-b70f3a82-58a1-4f94-ba54-5e6c62150217
bp-e9e1867d-b51d-4faf-b78f-e8c4e2150223
bp-226e04b3-fe8b-4a79-a09a-d22d62150309
bp-354fb1ab-8c1e-40ef-9fb1-0424d2150312
bp-caa55c01-4f91-4872-a0a5-433372150424
bp-dd7a9163-b572-41ac-aa5f-90e4a2150609

Another user bp-0533287f-8076-4c70-96af-065262150407
"I was trying to send a simple email with an attachment (a zipped folder about 1.5M). The msg was delivered it failed to save & asked if I wanted to try save again, I said yes, then it tried & crashed.
Actually, I've been wanting to ask you folk this: Lately, I begin to notice the time from pressing 'SEND' to having the mail put into the Send folder is taking very long, sometimes upwards of half a minute. I have whittled the Send box way down already, didn't work. Looking for suggestions. "

Slightly different (perhaps needs a new bug?)
bp-f02fc9a5-9a19-4e35-8deb-bd0d72150311 linux
" Keeps asking for password. Does not quit but only goes to sleep and I have to kill it before I can start over. Same problem for last several updates."
 0 	libxul.so	NS_CycleCollectorSuspect3
1 	libxul.so	nsArrayCC::Release
2 	libxul.so	nsImapMailCopyState::~nsImapMailCopyState
3 	libxul.so	nsImapMailCopyState::~nsImapMailCopyState

Comment 5

[Kent James (:rkent)]

This bug needs to proxy URL releases to the main thread in the IMAP protocol.

Comment 6

[Kent James (:rkent)]

Attachment: Proxy release nsCOMPtrs in nsMsgMailNewsUrl

This is one of the few top crashers in recent betas that have a known possible solution, so let's try a fix.

Comment 7

[Jorg K (CEST = GMT+2)]

Comment on attachment 8846343 [details] [diff] [review]
Proxy release nsCOMPtrs in nsMsgMailNewsUrl

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

Looks good, but I think I'm the wrong reviewer here since I don't really know what's going on. Could you please explain this a bit more, also for my education.

::: mailnews/base/util/nsMsgMailNewsUrl.cpp
@@ +52,5 @@
>  nsMsgMailNewsUrl::~nsMsgMailNewsUrl()
>  {
>    PR_FREEIF(m_errorMessage);
> +
> +  // In IMAP this url is created and destroyed on the imap thread,

Nit: URL.

@@ +61,5 @@
> +  NS_ReleaseOnMainThread(mMimeHeaders.forget());
> +  NS_ReleaseOnMainThread(m_searchSession.forget());
> +  NS_ReleaseOnMainThread(mMsgHeaderSink.forget());
> +
> +  nsTObserverArray<nsCOMPtr<nsIUrlListener> >::ForwardIterator iter(mUrlListeners);

Nit. Please lose the space <nsCOMPtr<nsIUrlListener> > (I know, it's copied).

Comment 8

[Kent James (:rkent)]

Here's more comments, as requested, on the why.

Many of the crash reports such as bp-419c05f4-785c-48f5-89e9-f0c3d2170310 are crashing on at nsXPCWrappedJS::Release() during the release of nsMsgMailNewsUrl::~nsMsgMailNewsUrl()  That means that there is an instance variable on the URL that must point to a JS-implemented component. Likely this is one of the URL listeners, but I could find examples of other components also being created in JS. JS XPCOM components are not allowed to release off of the main thread, and have a release assert if you try to do that.

So what I did is force release of any of the nsCOMPtr member variables in the URL on the main thread, unless I was absolutely sure it was implemented in C++.

Comment 9

[Jorg K (CEST = GMT+2)]

Thanks. Should be good to go then.

Comment 10

[Kent James (:rkent)]

https://hg.mozilla.org/comm-central/rev/3d0813e2e1bd

Comment 11

[Wayne Mery (:wsmwk)]

Hard to convey how excited I am for this patch! (and the other up and coming patches, like proxy)
I can't wait to see the crash-stats.

However, as is typical, because of low crash rates on development channels [1] we likely won't be able to confirm the effectiveness until it hits beta. And I think it unlikely I can scare up a user who can reproduce. So after a week or two on auroa and nightly, I'd like to see this uplifted to beta. 

[1] 
* nsXPCWrappedJS::Release only 11 in 3 months https://crash-stats.mozilla.com/signature/?product=Thunderbird&release_channel=%21release&release_channel=%21beta&signature=nsXPCWrappedJS%3A%3ARelease&date=%3E%3D2016-12-12T00%3A16%3A46.000Z&date=%3C2017-03-12T00%3A16%3A46.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_sort=-date&page=1#reports
* NS_CycleCollectorSuspect3 only 27 in 3 months https://crash-stats.mozilla.com/signature/?product=Thunderbird&release_channel=%21release&release_channel=%21beta&signature=NS_CycleCollectorSuspect3&date=%3E%3D2016-12-12T00%3A17%3A26.000Z&date=%3C2017-03-12T00%3A17%3A26.000Z&_columns=date&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=reason&_columns=address&_sort=-date&page=1#reports

Comment 12

[Jorg K (CEST = GMT+2)]

Comment on attachment 8846343 [details] [diff] [review]
Proxy release nsCOMPtrs in nsMsgMailNewsUrl

If Wayne is excited, I guess we want uplifts ;-)

Comment 13

[Jorg K (CEST = GMT+2)]

Aurora (TB 54):
https://hg.mozilla.org/releases/comm-aurora/rev/9e504bb64b31e2f5c54c8db0597166d3a988c8ca

Comment 14

[Jorg K (CEST = GMT+2)]

Beta (TB 53):
https://hg.mozilla.org/releases/comm-beta/rev/95ccf0384992835cbb49d883367beb0d3c2183d7

Comment 15

[Jorg K (CEST = GMT+2)]

TB 52 ESR:
https://hg.mozilla.org/releases/comm-esr52/rev/6f372c54bde8c4b0d69e9d038a712c0f1eaece08