1857561 - Crash in [@ nsRefreshDriver::Tick]

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Andrew McCreight [:mccr8]]

Crash report: https://crash-stats.mozilla.org/report/index/3371625b-d926-4185-911f-51e9b0231006

Reason: EXC_BAD_ACCESS / KERN_INVALID_ADDRESS

Top 10 frames of crashing thread:

0  XUL  nsRefreshDriver::Tick  layout/base/nsRefreshDriver.cpp:2853
1  XUL  mozilla::RefreshDriverTimer::TickDriver  layout/base/nsRefreshDriver.cpp:359
1  XUL  mozilla::RefreshDriverTimer::TickRefreshDrivers  layout/base/nsRefreshDriver.cpp:337
2  XUL  mozilla::RefreshDriverTimer::Tick  layout/base/nsRefreshDriver.cpp:353
3  XUL  mozilla::VsyncRefreshDriverTimer::RunRefreshDrivers  layout/base/nsRefreshDriver.cpp:924
3  XUL  mozilla::VsyncRefreshDriverTimer::TickRefreshDriver  layout/base/nsRefreshDriver.cpp:834
4  XUL  mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsyncTimerOnMainThread  layout/base/nsRefreshDriver.cpp:578
4  XUL  mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync  layout/base/nsRefreshDriver.cpp:535
5  XUL  mozilla::dom::VsyncMainChild::RecvNotify  dom/ipc/VsyncMainChild.cpp:66
6  XUL  mozilla::dom::PVsyncChild::OnMessageReceived  ipc/ipdl/PVsyncChild.cpp:220

I went through null deref crashes with nsRefreshDriver in the proto signature, and I found this one. There's only a single instance of it, but maybe this is a regression from bug 1807253, too?

The crash is on this line, so maybe mPresContext is null?
if (mPresContext->IsRoot() && XRE_IsContentProcess() &&

Comment 1

[Andrew McCreight [:mccr8]]

Now there are about a dozen crashes like this. So far they are present only in the 20231006092133 build. Maybe this is a "regression" from bug 1857115? Like that other patch fixed some crashes, but they turned into this one?

Comment 2

[Daniel Holbert [:dholbert]]

I think you're right, yeah; we're just crashing slightly later now. I'm writing a patch now, to just hoist out the null-checks that bug 1857115 added and bail entirely from Tick() if we've been disconnected.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: Bug 1857561: Make nsRefreshDriver::Tick gracefully bail if a resize observer destroys the PresContext. r?#layout,surkov

Note, I'm deleting the null-check from UpdateIntersectionObservations, since
I'm essentially hoisting it up one level (and I don't think there's anything
that could null out mPresContext in the intervening code).

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1857115

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bcfcd8b57192
Make nsRefreshDriver::Tick gracefully bail if a resize observer destroys the PresContext. r=surkov,emilio

Comment 6

[Cristian Tuns]

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

Comment 7

[Andrew McCreight [:mccr8]]

Here's another signature that looks like the same basic issue: [@ nsPresContext::NotifyDOMContentFlushed ]. bp-ac2a626b-16ff-43d5-bc7e-b1b290231009

It looks like it will also be fixed by this patch.