Closed Bug 1389184 Opened 3 years ago Closed 3 years ago

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

Categories

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

56 Branch
x86
Windows
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- fixed
firefox57 --- fixed

People

(Reporter: philipp, Assigned: bkelly)

References

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

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.
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.
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
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.
Attachment #8895985 - Flags: review?(bugmail)
Attachment #8895985 - Flags: review?(bugmail) → review+
Pushed by bkelly@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9a92b98a965
Avoid crashing in worker MemoryReporter if the GlobalScope is nullptr. r=asuth
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
Attachment #8895985 - Flags: approval-mozilla-beta?
https://hg.mozilla.org/mozilla-central/rev/b9a92b98a965
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
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.
Attachment #8895985 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.