1389184 - Crash in mozilla::dom::workers::WorkerPrivate::MemoryReporter::CollectReportsRunnable::WorkerRun

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-09215865-548d-49fa-b4cd-0c9050170810.
=============================================================
Crashing Thread (70), Name: DOM Worker
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::dom::workers::WorkerPrivate::MemoryReporter::CollectReportsRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) 	dom/workers/WorkerPrivate.cpp:2602
1 	xul.dll 	mozilla::dom::workers::WorkerRunnable::Run() 	dom/workers/WorkerRunnable.cpp:374
2 	xul.dll 	mozilla::dom::workers::WorkerPrivate::ProcessAllControlRunnablesLocked() 	dom/workers/WorkerPrivate.cpp:5550
3 	xul.dll 	mozilla::dom::workers::WorkerPrivate::RunCurrentSyncLoop() 	dom/workers/WorkerPrivate.cpp:5900
4 	xul.dll 	`anonymous namespace'::LoadAllScripts 	dom/workers/ScriptLoader.cpp:2127
5 	xul.dll 	mozilla::dom::workers::scriptloader::LoadMainScript(mozilla::dom::workers::WorkerPrivate*, nsAString const&, mozilla::dom::workers::WorkerScriptType, mozilla::ErrorResult&) 	dom/workers/ScriptLoader.cpp:2245
6 	xul.dll 	`anonymous namespace'::CompileScriptRunnable::WorkerRun 	dom/workers/WorkerPrivate.cpp:601
7 	xul.dll 	mozilla::dom::workers::WorkerRunnable::Run() 	dom/workers/WorkerRunnable.cpp:374
8 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1446
9 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/threads/nsThreadUtils.cpp:480
10 	xul.dll 	mozilla::dom::workers::WorkerPrivate::DoRunLoop(JSContext*) 	dom/workers/WorkerPrivate.cpp:5170
11 	xul.dll 	`anonymous namespace'::WorkerThreadPrimaryRunnable::Run 	dom/workers/RuntimeService.cpp:2920
12 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1446
13 	xul.dll 	NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/threads/nsThreadUtils.cpp:480
14 	xul.dll 	mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:369
15 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:319
16 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:299
17 	xul.dll 	nsThread::ThreadFunc(void*) 	xpcom/threads/nsThread.cpp:506
18 	nss3.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:397
19 	nss3.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:95
20 	ucrtbase.dll 	thread_start<unsigned int (__stdcall*)(void*)> 	
21 	kernel32.dll 	BaseThreadInitThunk 	
22 	mozglue.dll 	patched_BaseThreadInitThunk 	mozglue/build/WindowsDllBlocklist.cpp:815
23 	ntdll.dll 	__RtlUserThreadStart 	
24 	ntdll.dll 	_RtlUserThreadStart

this crash signature is newly showing up in early data from the staged rollout of 56.0b and in a codepath that was last touched in bug 1382768.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Its trying to get a memory report while starting a new worker.  I think we just need to nullptr check the return from GlobalScope() here.

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Avoid crashing in worker MemoryReporter if the GlobalScope is nullptr. r=asuth

https://treeherder.mozilla.org/#/jobs?repo=try&revision=13f7d1edb4617de67f5af55c53bdc3eaefaf5061

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8895985 [details] [diff] [review]
Avoid crashing in worker MemoryReporter if the GlobalScope is nullptr. r=asuth

Check the GlobalScope for nullptr before trying to use it.  This can happen if we get a MemoryReporter runnable while running the sync loop during initial worker load.  The global scope is not created until the first ScriptExecutor.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9a92b98a965
Avoid crashing in worker MemoryReporter if the GlobalScope is nullptr. r=asuth

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8895985 [details] [diff] [review]
Avoid crashing in worker MemoryReporter if the GlobalScope is nullptr. r=asuth

Approval Request Comment
[Feature/Bug causing the regression]: MemoryReporter interacting with worker thread startup.
[User impact if declined]: Crashes being observed in beta.
[Is this code covered by automated tests?]: Its racy so I don't have a test that provokes the behavior.  The fix was identified via code inspection.
[Has the fix been verified in Nightly?]: It has not landed on m-c yet.  We don't have STR, though, so we cannot do manual verification.
[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?]: Minimal risk
[Why is the change risky/not risky?]: This code simply adds a nullptr check.
[String changes made/needed]: None

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b9a92b98a965

Comment 7

[Gerry Chang [:gchang]]

Comment on attachment 8895985 [details] [diff] [review]
Avoid crashing in worker MemoryReporter if the GlobalScope is nullptr. r=asuth

Fix a crash. Beta56+. Should be in 56.0b3.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ee0bdebf299a