Closed Bug 1133892 Opened 6 years ago Closed 4 years ago

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

Categories

(MailNews Core :: Networking: IMAP, defect)

defect
Not set
critical

Tracking

(thunderbird_esr5253+ fixed, thunderbird53 fixed, thunderbird54 fixed, thunderbird55 fixed)

RESOLVED FIXED
Thunderbird 55.0
Tracking Status
thunderbird_esr52 53+ fixed
thunderbird53 --- fixed
thunderbird54 --- fixed
thunderbird55 --- fixed

People

(Reporter: wsmwk, Assigned: rkent)

References

(Blocks 1 open bug)

Details

(Keywords: crash, dataloss, topcrash-thunderbird, Whiteboard: [firefox crashes see http://mzl.la/1IRPjWX ])

Crash Data

Attachments

(1 file)

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)
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
(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.
Summary: Thunderbird 31 crash in NS_CycleCollectorSuspect3 and nsXPCWrappedJS::Release(). Release off main thread → Thunderbird 31 crash in NS_CycleCollectorSuspect3 and nsXPCWrappedJS::Release(). Release off main thread. [imap]
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
Keywords: dataloss
See Also: → 870864
Summary: Thunderbird 31 crash in NS_CycleCollectorSuspect3 and nsXPCWrappedJS::Release(). Release off main thread. [imap] → Thunderbird 31 crash in NS_CycleCollectorSuspect3 and nsXPCWrappedJS::Release() via CopyListener::~CopyListener (nsImapMailCopyState::~nsImapMailCopyState and nsMsgComposeAndSend::~nsMsgComposeAndSend). Release off main thread. [imap]
Whiteboard: [firefox crashes see http://mzl.la/1IRPjWX ]
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
Crash Signature: [@ NS_CycleCollectorSuspect3] [@ nsXPCWrappedJS::Release()] → [@ NS_CycleCollectorSuspect3] [@ nsXPCWrappedJS::Release()] [@ nsXPCWrappedJS::Release]
Blocks: 1251932
This bug needs to proxy URL releases to the main thread in the IMAP protocol.
See Also: → 1255903
Blocks: 1303222
Depends on: 1299920
This is one of the few top crashers in recent betas that have a known possible solution, so let's try a fix.
Assignee: nobody → rkent
Status: NEW → ASSIGNED
Attachment #8846343 - Flags: review?(jorgk)
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).
Attachment #8846343 - Flags: review?(jorgk) → review+
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++.
Thanks. Should be good to go then.
https://hg.mozilla.org/comm-central/rev/3d0813e2e1bd
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 55.0
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 on attachment 8846343 [details] [diff] [review]
Proxy release nsCOMPtrs in nsMsgMailNewsUrl

If Wayne is excited, I guess we want uplifts ;-)
Attachment #8846343 - Flags: approval-comm-esr52?
Attachment #8846343 - Flags: approval-comm-beta?
Attachment #8846343 - Flags: approval-comm-aurora?
Attachment #8846343 - Flags: approval-comm-aurora? → approval-comm-aurora+
Attachment #8846343 - Flags: approval-comm-beta? → approval-comm-beta+
Attachment #8846343 - Flags: approval-comm-esr52? → approval-comm-esr52+
See Also: → 1175168
You need to log in before you can comment on or make changes to this bug.