release off main thread crash in nsXPCWrappedJS::Release() via nsUpdateProcessor::~nsUpdateProcessor()

RESOLVED FIXED in Firefox 36

Status

()

--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: alice0775, Assigned: rstrong)

Tracking

({crash})

Trunk
mozilla38
x86
Windows 7
crash
Points:
---

Firefox Tracking Flags

(firefox35 wontfix, firefox36 fixed, firefox37 fixed, firefox38 fixed)

Details

(Whiteboard: [tbird crash], crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
This bug was filed from the Socorro interface and is 
report bp-dba7aca3-756a-4b78-92be-25d062140726.
=============================================================

Crash after daily update from Help > About Nightly
(Reporter)

Updated

4 years ago
OS: Windows NT → Windows 7

Comment 1

4 years ago
The nsCOMPtr<nsIUpdate> in nsUpdateProcessor is the culprit here.
(Reporter)

Comment 2

4 years ago
Aurora33.0a2 also crashed
bp-67a29718-895a-45a4-bd81-daf4a2140805
Version: 34 Branch → 33 Branch

Comment 3

4 years ago
Reproduce on:
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:36.0) Gecko/20100101 Firefox/36.0

bp-e90a1577-4586-4723-b667-9dd062141014
bp-6358fbd4-f1d3-414b-b7a1-49b592141013

Comment 4

4 years ago
Thunderbird dev builds
bp-cc16ba46-4a7d-40d7-9284-77fc22141006
bp-a943a723-8a97-4385-9ab1-8288f2141020
bp-6a838c1d-3bcc-4936-ba84-6f8832141018
 0 	xul.dll	nsXPCWrappedJS::Release()	js/xpconnect/src/XPCWrappedJS.cpp
1 	xul.dll	nsUpdateProcessor::~nsUpdateProcessor()	toolkit/xre/nsUpdateDriver.cpp
2 	xul.dll	nsUpdateProcessor::Release()	toolkit/xre/nsUpdateDriver.cpp
3 	xul.dll	nsRunnableMethodImpl<tag_nsresult ( mozilla::net::BackgroundFileSaverStreamListener::*)(void), void, 1>::`scalar deleting destructor'(unsigned int)	
4 	xul.dll	AsyncLatencyLogger::Release()	netwerk/base/src/Dashboard.cpp
5 	xul.dll	nsThread::ProcessNextEvent(bool, bool*)	xpcom/threads/nsThread.cpp
6 	xul.dll	NS_ProcessNextEvent(nsIThread*, bool)	xpcom/glue/nsThreadUtils.cpp
7 	xul.dll	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)	ipc/glue/MessagePump.cpp
Whiteboard: [tbird crash]

Updated

4 years ago
Component: General → Application Update
Product: Core → Toolkit
Summary: crash in nsXPCWrappedJS::Release() → crash in nsXPCWrappedJS::Release() via nsUpdateProcessor::~nsUpdateProcessor()
Version: 33 Branch → Trunk

Comment 5

4 years ago
I was testing TB34beta update to TB36beta Help > About > Check for updates

me during startup bp-68c79503-d30b-4c7c-aadf-641032150216
0 	xul.dll	nsXPCWrappedJS::Release()	js/xpconnect/src/XPCWrappedJS.cpp
1 	xul.dll	nsXPTCStubBase::Release()	xpcom/reflect/xptcall/xptcall.cpp
2 	xul.dll	nsUpdateProcessor::~nsUpdateProcessor()	toolkit/xre/nsUpdateDriver.cpp
3 	xul.dll	nsUpdateProcessor::Release()	toolkit/xre/nsUpdateDriver.cpp
4 	xul.dll	nsRunnableMethodImpl<tag_nsresult ( mozilla::widget::AudioSession::*)(void), void, 1>::~nsRunnableMethodImpl<tag_nsresult ( mozilla::widget::AudioSession::*)(void), void, 1>()	
5 	xul.dll	nsRunnableMethodImpl<void ( CameraPermissionRequest::*)(void), void, 1>::`vector deleting destructor'(unsigned int)	
6 	xul.dll	nsTokenEventRunnable::Release()	netwerk/base/src/nsUDPSocket.cpp 

For nightly builds, approx 50% with this signature have this stack, which ranks it in the top 10.
Summary: crash in nsXPCWrappedJS::Release() via nsUpdateProcessor::~nsUpdateProcessor() → release off main thread crash in nsXPCWrappedJS::Release() via nsUpdateProcessor::~nsUpdateProcessor() during startup
Whiteboard: [tbird crash] → [tbird crash][startupcrash]
How were you able to get this to crash on startup?
Flags: needinfo?(vseerror)
Created attachment 8565786 [details] [diff] [review]
patch that can be uplifted

Brian, I haven't been able to reproduce but it is obvious that the reference to the nsIUpdate doesn't need to be kept. This patch could be uplifted to branches if you think it will help and if you do I can put together a patch to clean it up further for nightly and aurora but that will require the uuid for nsIUpdateManager.
Attachment #8565786 - Flags: review?(netzen)

Comment 9

4 years ago
1. installed 34.0beta
2. started up to test profile (unknown which version of Thunderbird I last ran in it)
3. help | about | check for updates
4. pick update to 36.0 beta
5. version 36 installs and starts
6. bp-68c79503-d30b-4c7c-aadf-641032150216 

NOTE: the report cites version 34 but should it not be version 36?
Right after that I made a second attempt with the same steps, was unable to reproduce. 

Firefox crashes with this stack (not all startup)
bp-e90a1577-4586-4723-b667-9dd062141014
bp-6358fbd4-f1d3-414b-b7a1-49b592141013
bp-67a29718-895a-45a4-bd81-daf4a2140805


Not to confuse, but there are other crashes in this league, but with different immediate cuase
this is a crash of mine on Feb 8, different stack. 
At the time I was not at the computer.
bp-a3da1435-0f95-46de-862c-5dcd62150208
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)
For my own clarity, does this crash happen after clicking the button in the about window?
Also, could you provide the text of the button in the about window or a screenshot?

Comment 12

4 years ago
Created attachment 8565799 [details]
TB34beta-check.png

(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #10)
> For my own clarity, does this crash happen after clicking the button in the
> about window?

crash was after clicking *restart*
Flags: needinfo?(vseerror)
I'm quite certain it isn't during startup which is supported by the report being for 34 instead of 36.
Summary: release off main thread crash in nsXPCWrappedJS::Release() via nsUpdateProcessor::~nsUpdateProcessor() during startup → release off main thread crash in nsXPCWrappedJS::Release() via nsUpdateProcessor::~nsUpdateProcessor()
Whiteboard: [tbird crash][startupcrash] → [tbird crash]
Created attachment 8566094 [details] [diff] [review]
patch that can be uplifted
Assignee: nobody → robert.strong.bugs
Attachment #8565786 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8565786 - Flags: review?(netzen)
Attachment #8566094 - Flags: feedback?(netzen)
Comment on attachment 8566094 [details] [diff] [review]
patch that can be uplifted

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

Worth a try I think.
Attachment #8566094 - Flags: feedback?(netzen) → feedback+
Comment on attachment 8566094 [details] [diff] [review]
patch that can be uplifted

Brian, sorry for the additional request.... spohl isn't around
Attachment #8566094 - Flags: review?(netzen)
Attachment #8566094 - Flags: review?(netzen) → review+
https://hg.mozilla.org/mozilla-central/rev/b596d5ffe50f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
status-firefox35: --- → wontfix
status-firefox36: --- → wontfix
status-firefox37: --- → affected
Comment on attachment 8566094 [details] [diff] [review]
patch that can be uplifted

Crash reports for this signature have not been present on Nightly since this landed.

Approval Request Comment
[Feature/regressing bug #]: Bug 307181 and exposed by shutdownhang crashes
[User impact if declined]: Continued to crash during shutdown when staging an update
[Describe test coverage new/current, TreeHerder]: the code is exercised and tested by both xpcshell and mochitest-chrome tests.
[Risks and why]: Minimal due to the patch being a simple change of not keeping a reference to the object. Also, I have found no indication that the crash has moved to a different signature.
[String/UUID change made/needed]: None
Attachment #8566094 - Flags: approval-mozilla-beta?
Attachment #8566094 - Flags: approval-mozilla-aurora?
Comment on attachment 8566094 [details] [diff] [review]
patch that can be uplifted

This will make it into aurora with the merge so removing that request.
Attachment #8566094 - Flags: approval-mozilla-aurora?
Comment on attachment 8566094 [details] [diff] [review]
patch that can be uplifted

Taking this crash fix related to updates in 37. Beta+
Attachment #8566094 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
status-firefox36: wontfix → affected
Comment on attachment 8566094 [details] [diff] [review]
patch that can be uplifted

Approval Request Comment
[Feature/regressing bug #]: Bug 307181 and exposed by shutdownhang crashes
[User impact if declined]: Continued to crash during shutdown when staging an update
[Describe test coverage new/current, TreeHerder]: the code is exercised and tested by both xpcshell and mochitest-chrome tests.
[Risks and why]: Minimal due to the patch being a simple change of not keeping a reference to the object. Also, I have found no indication that the crash has moved to a different signature.
[String/UUID change made/needed]: None
Attachment #8566094 - Flags: approval-mozilla-release?
Attachment #8566094 - Flags: approval-mozilla-release? → approval-mozilla-release+
Pushed to mozilla-release
https://hg.mozilla.org/releases/mozilla-release/rev/3cd0f44138da
status-firefox36: affected → fixed
Verified in Socorro for the past 4 weeks, and this looks good so far:
39.0a1 - 1 crash for TB (not via nsUpdateProcessor)
38.0a2 - 0 crashes
38.0a1 (builds after Feb 19) - 0 crashes
37.0b2 (and older) - 2 crashes (not via nsUpdateProcessor)
36.0.1 - need more time to gather data

[1] - https://crash-stats.mozilla.com/report/list?range_unit=days&range_value=28&signature=nsXPCWrappedJS%3A%3ARelease%28%29#tab-sigsummary
You need to log in before you can comment on or make changes to this bug.