1701190 - Crash in [@ mozilla::dom::WorkerDebugger::ReportPerformanceInfo]

Status: RESOLVED FIXED

(Core :: DOM: Workers, defect, P1)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This hit rarely before but more frequent since March 18/19 (Firefox 88.0a1)

Crash report: https://crash-stats.mozilla.org/report/index/2131b145-b312-4cff-b16f-f19750210326

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::WorkerDebugger::ReportPerformanceInfo dom/workers/WorkerDebugger.cpp:512
1 xul.dll mozilla::CollectPerformanceInfo toolkit/components/perfmonitoring/PerformanceUtils.cpp:38
2 xul.dll mozilla::dom::ContentChild::RecvRequestPerformanceMetrics dom/ipc/ContentChild.cpp:1453
3 xul.dll mozilla::dom::PContentChild::OnMessageReceived ipc/ipdl/PContentChild.cpp:9168
4 xul.dll mozilla::ipc::MessageChannel::DispatchMessage ipc/glue/MessageChannel.cpp:2078
5 xul.dll mozilla::TaskController::DoExecuteNextTaskOnlyMainThreadInternal xpcom/threads/TaskController.cpp:754
6 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1155
7 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:109
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:328
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:310

Comment 1

[Jens Stutte [:jstutte]]

windowID = top->GetCurrentWindowContext()->OuterWindowId();
     00007FFCC58E0C02 48 8B 83 58 03 00 00 mov         rax,qword ptr [rbx+358h]  
->   RAX = 0000000000000000
>>>> 00007FFCC58E0C09 4C 8B B0 70 01 00 00 mov         r14,qword ptr [rax+170h]  

means probably that top->GetCurrentWindowContext() returns a nullptr. It seems from here that it must be expected to be nullptr sometimes, instead.

Bug 1646505 changed that specific line, contributing probably to the higher frequency. There might be chances that this is not the only potentially broken pointer access in ReportPerformanceInfo, though.

Comment 2

[Andreas Farre [:farre]]

Attachment: Bug 1701190 - Make sure that a current window context exists.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03b025c559e7
Make sure that a current window context exists. r=dom-worker-reviewers,asuth

Comment 4

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/03b025c559e7

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Release approval as a possible ride-along candidate.

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 9214103 [details]
Bug 1701190 - Make sure that a current window context exists.

Beta/Release Uplift Approval Request

• User impact if declined: Crashes when performance information is gathered for about:performance/about:processes and browser tabs (/ iframes?) are being closed.
• 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 patch is only a null-check/guard and is extremely low-risk.
• String changes made/needed:

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

(I requested release approval.)

Comment 8

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9214103 [details]
Bug 1701190 - Make sure that a current window context exists.

Approved for 88.0rc2, thanks.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/11ca7ae0ab47