Crash in mozilla::EventQueue::`scalar deleting destructor''

RESOLVED FIXED in Firefox 57

Status

()

defect
P3
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: philipp, Assigned: billm)

Tracking

({crash, regression})

57 Branch
mozilla58
x86
Windows
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57 fixed, firefox58 fixed)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-5ed9afbd-3dc9-4132-9d27-626b70170930.
=============================================================
Crashing Thread (7), Name: Socket Thread
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::EventQueue::`scalar deleting destructor'(unsigned int) 	xpcom/threads/nsThreadPool.cpp:61
1 	xul.dll 	mozilla::psm::StopSSLServerCertVerificationThreads() 	security/manager/ssl/SSLServerCertVerification.cpp:198
2 	xul.dll 	mozilla::net::nsSocketTransportService::Run() 	netwerk/base/nsSocketTransportService2.cpp:1068
3 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1039
4 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/threads/nsThreadUtils.cpp:521
5 	xul.dll 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:338
6 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:319
7 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:299
8 	xul.dll 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp:427
9 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:397
10 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:95
11 	ucrtbase.dll 	_o__CIpow 	
12 	kernel32.dll 	BaseThreadInitThunk 	
13 	mozglue.dll 	patched_BaseThreadInitThunk 	mozglue/build/WindowsDllBlocklist.cpp:824
14 	ntdll.dll 	__RtlUserThreadStart 	
15 	ntdll.dll 	_RtlUserThreadStart

crashes during shutdown with this signature are newly showing up in 57 - so far only from 32bit firefox versions on windows 7 & 8 and all with "MOZ_RELEASE_ASSERT(IsEmpty())".

the mejority of these crashes go through mozilla::psm::StopSSLServerCertVerificationThreads() in the stack, a few of them also through mozilla::dom::indexedDB::`anonymous namespace'::QuotaClient::ShutdownWorkThreads.
Hi JC,

The backtrace is in the PSM module.
Could you find someone to investigate this bug?
Flags: needinfo?(jjones)
Thanks, Ethan.

Adding Franziskus and David --

David: Can you dig into this this morning?
Flags: needinfo?(jjones) → needinfo?(dkeeler)
From what I can tell, necko is shutting down the cert verification thread pool. The assertion that's failing is that the thread pool's event queue's queue isn't empty. This doesn't make much sense because we just shut it down (and indeed, there aren't any ssl verification threads in the stack traces) and necko is in shutdown so it shouldn't be possible for it to be dispatching events to the nonexistent cert verification thread pool. Looks like bug 1382922 added this release assertion, but this behavior may have been present before then. Anyway, needinfo to :billm in the hopes that he might have some ideas.
Blocks: 1382922
Flags: needinfo?(dkeeler) → needinfo?(wmccloskey)
I think this is a preexisting problem. nsThreadPool uses its own event queue, different from the event queue of any of the threads in the pool. A normal nsThread will ensure that all events are processed before it shuts down. It looks like nsThreadPool has no such protection. We should probably add something to do that.

I think the only change with bug 1382922 was to make this a release assertion. It might be a good idea to turn this back to a DEBUG assertion. I'm not sure we'll have time to fix it in the near future.
Component: General → XPCOM
Flags: needinfo?(wmccloskey)
Lets fix the assert for 57.
Flags: needinfo?(wmccloskey)
Priority: -- → P3
Posted patch patchSplinter Review
See comment 4.
Assignee: nobody → wmccloskey
Flags: needinfo?(wmccloskey)
Attachment #8917180 - Flags: review?(nfroyd)
Attachment #8917180 - Flags: review?(nfroyd) → review+
Pushed by wmccloskey@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fb8794293bc6
Make Queue emptiness assertion DEBUG-only (r=froydnj)
https://hg.mozilla.org/mozilla-central/rev/fb8794293bc6
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
New crash in 57, please request Beta approval when you get a chance.
Flags: needinfo?(nfroyd)
Comment on attachment 8917180 [details] [diff] [review]
patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1382922
[User impact if declined]: Crashes
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Switching asserts from release mode to debug mode isn't going to impact anything.
[String changes made/needed]: None.
Flags: needinfo?(nfroyd)
Attachment #8917180 - Flags: approval-mozilla-beta?
Comment on attachment 8917180 [details] [diff] [review]
patch

debug assert is better, Beta57+
Attachment #8917180 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Nathan Froyd [:froydnj] from comment #10)
> [Is this code covered by automated tests?]: Yes
> [Has the fix been verified in Nightly?]: No
> [Needs manual test from QE? If yes, steps to reproduce]: No

Setting qe-verify- based on Nathan's assessment on manual testing and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.