Closed Bug 1513304 Opened 1 year ago Closed 1 year ago

Crash in mozilla::dom::WorkerDebugger::ReportPerformanceInfo

Categories

(Toolkit :: Performance Monitoring, defect, critical)

Unspecified
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- fixed
firefox66 --- fixed

People

(Reporter: marcia, Assigned: tarek)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

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

=============================================================
Flags: needinfo?(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
Flags: needinfo?(tarek)
Assignee: nobody → tarek
Tarek, I don't see a patch on this bug. Were you able to put together a patch for this?
Flags: needinfo?(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.
Flags: needinfo?(tarek)
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.
On 66 nightly crashes the last crashes were in 20181215214208.
Pushed by rvandermeulen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/af611c004da2
check that the nsIURI pointer is not null - r=baku
https://hg.mozilla.org/mozilla-central/rev/af611c004da2
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66
Please request Beta approval on this when you get a chance.
Flags: needinfo?(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:
Flags: needinfo?(tarek)
Attachment #9032901 - Flags: approval-mozilla-beta?
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.
Attachment #9032901 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.