1513304 - Crash in mozilla::dom::WorkerDebugger::ReportPerformanceInfo

Status: RESOLVED FIXED

(Toolkit :: Performance Monitoring, defect)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-3f28e154-96c9-4319-b20b-0d98a0181211.
=============================================================

Seen while looking at nightly crash data - crashes started using 20181210220553 - 2 crashes so far both on 10.14: https://bit.ly/2PvuB6h

Possible regression range based on Build ID: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=88d304c633b6091a21fc64290256ef3fb51b7421&tochange=13f891b92db19ea8ec85ef329eff7a793c8b368f

Code was also touched in Bug 1497124. ni on :tarek to see if he can help

Top 10 frames of crashing thread:

0 XUL mozilla::dom::WorkerDebugger::ReportPerformanceInfo dist/include/nsIURI.h:48
1 XUL mozilla::CollectPerformanceInfo toolkit/components/perfmonitoring/PerformanceUtils.cpp:38
2 XUL mozilla::dom::ContentChild::RecvRequestPerformanceMetrics dom/ipc/ContentChild.cpp:1325
3 XUL mozilla::dom::PContentChild::OnMessageReceived ipc/ipdl/PContentChild.cpp:6109
4 XUL mozilla::ipc::MessageChannel::DispatchMessage ipc/glue/MessageChannel.cpp:2159
5 XUL mozilla::ipc::MessageChannel::MessageTask::Run ipc/glue/MessageChannel.cpp:1935
6 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1157
7 XUL NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:468
8 XUL mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:110
9 XUL nsBaseAppShell::Run ipc/chromium/src/base/message_loop.cc:314

=============================================================

Comment 1

[Tarek Ziadé (:tarek)]

Thanks Marcia,

I am not sure how to reproduce this yet, but in any case we should assert that scriptURI is not null at

https://searchfox.org/mozilla-central/source/dom/workers/WorkerDebugger.cpp#460

will push a patch later today

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

Tarek, I don't see a patch on this bug. Were you able to put together a patch for this?

Comment 3

[Tarek Ziadé (:tarek)]

After seeing the small number of crashes and no crash the day after, I ended up post-poning this work for when I am back from PTO in January. (I was on PTO when it happened) But I see that we had a few more since then, I will try to do this tomorrow.

Comment 4

[Tarek Ziadé (:tarek)]

Attachment: Bug 1513304 - check that the nsIURI pointer is not null - r?baku

Verify that the script URI is not null before using it.
If it's the case, we can't really continue because that url
identifies the worker in the metrics.

Comment 5

[Marcia Knous [:marcia]]

On 66 nightly crashes the last crashes were in 20181215214208.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/af611c004da2
check that the nsIURI pointer is not null - r=baku

Comment 7

[Bogdan Tara[:bogdan_tara | bogdant]]

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

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Please request Beta approval on this when you get a chance.

Comment 9

[Tarek Ziadé (:tarek)]

Comment on attachment 9032901 [details]
Bug 1513304 - check that the nsIURI pointer is not null - r?baku

[Beta/Release Uplift Approval Request]

Feature/Bug causing the regression: about:performance

User impact if declined: possible crash when using about:performance. We think that only happens on shutdown when a Worker starts to free its allocations

Is this code covered by automated tests?: No

Has the fix been verified in Nightly?: Yes

Needs manual test from QE?: No

If yes, steps to reproduce: 

List of other uplifts needed: None

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): The change is not risky since I am introducing here an extra step to verify that I am not doing an UAF on an object.

String changes made/needed:

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9032901 [details]
Bug 1513304 - check that the nsIURI pointer is not null - r?baku

[Triage Comment]
Simple null check crash fix. Approved for 65.0b9.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/467d52576809