crash at null in [@ mozilla::ServoStyleSet::UpdateStylist]

RESOLVED FIXED in Firefox 63

Status

()

defect
RESOLVED FIXED
9 months ago
9 months ago

People

(Reporter: tsmith, Assigned: emilio)

Tracking

(Blocks 1 bug, {crash, regression, testcase})

unspecified
mozilla64
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox62 unaffected, firefox63+ fixed, firefox64 fixed)

Details

(crash signature)

Attachments

(3 attachments)

Reporter

Description

9 months ago
Posted file testcase.html
==13208==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7f3314ebc223 bp 0x7ffd7163a2d0 sp 0x7ffd71637280 T0)
==13208==The signal is caused by a WRITE memory access.
==13208==Hint: address points to the zero page.
    #0 0x7f3314ebc222 in core::sync::atomic::atomic_compare_exchange::h193b56c153d97e17 /checkout/src/libcore/sync/atomic.rs:1766
    #1 0x7f3314ebc222 in core::sync::atomic::AtomicUsize::compare_exchange::h91a3e64cda81338d /checkout/src/libcore/sync/atomic.rs:1231
    #2 0x7f3314ebc222 in atomic_refcell::AtomicBorrowRefMut::new::ha2c7e3ba3173e345 src/third_party/rust/atomic_refcell/src/lib.rs:194
    #3 0x7f3314ebc222 in _$LT$atomic_refcell..AtomicRefCell$LT$T$GT$$GT$::borrow_mut::h970f272970c98fb1 src/third_party/rust/atomic_refcell/src/lib.rs:97
    #4 0x7f3314ebc222 in style::gecko::data::PerDocumentStyleData::borrow_mut::h828b418b5058324e src/servo/components/style/gecko/data.rs:169
    #5 0x7f3314ebc222 in Servo_StyleSet_FlushStyleSheets src/servo/ports/geckolib/glue.rs:1529
    #6 0x7f330f68f9f0 in mozilla::ServoStyleSet::UpdateStylist() src/layout/style/ServoStyleSet.cpp:1466:5
    #7 0x7f330f7fcaa1 in UpdateStylistIfNeeded src/obj-firefox/dist/include/mozilla/ServoStyleSet.h:291:7
    #8 0x7f330f7fcaa1 in mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) src/layout/base/PresShell.cpp:4274
    #9 0x7f330f78cab6 in FlushPendingNotifications src/layout/base/nsIPresShell.h:577:5
    #10 0x7f330f78cab6 in nsRefreshDriver::Tick(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1900
    #11 0x7f330f79b75f in TickDriver src/layout/base/nsRefreshDriver.cpp:324:13
    #12 0x7f330f79b75f in mozilla::RefreshDriverTimer::TickRefreshDrivers(mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:299
    #13 0x7f330f79b391 in mozilla::RefreshDriverTimer::Tick(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:317:5
    #14 0x7f330f79debe in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:755:5
    #15 0x7f330f79debe in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:671
    #16 0x7f330f79dac0 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:571:9
    #17 0x7f331005612f in mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) src/layout/ipc/VsyncChild.cpp:78:16
    #18 0x7f3308a49e08 in mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PVsyncChild.cpp:167:20
    #19 0x7f33088d5836 in mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) src/obj-firefox/ipc/ipdl/PBackgroundChild.cpp:2280:28
    #20 0x7f330843048e in mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) src/ipc/glue/MessageChannel.cpp:2239:25
    #21 0x7f330842d3a4 in mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message&&) src/ipc/glue/MessageChannel.cpp:2166:17
    #22 0x7f330842ebfc in mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) src/ipc/glue/MessageChannel.cpp:2012:5
    #23 0x7f330842f258 in mozilla::ipc::MessageChannel::MessageTask::Run() src/ipc/glue/MessageChannel.cpp:2045:15
    #24 0x7f33074f5e0f in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1167:14
    #25 0x7f33074fcfa8 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:519:10
    #26 0x7f3308437f3a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
    #27 0x7f330838bb2c in RunInternal src/ipc/chromium/src/base/message_loop.cc:325:10
    #28 0x7f330838bb2c in RunHandler src/ipc/chromium/src/base/message_loop.cc:318
    #29 0x7f330838bb2c in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298
    #30 0x7f330f226eca in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:158:27
    #31 0x7f3312eb6e1f in XRE_RunAppShell() src/toolkit/xre/nsEmbedFunctions.cpp:944:22
    #32 0x7f330838bb2c in RunInternal src/ipc/chromium/src/base/message_loop.cc:325:10
    #33 0x7f330838bb2c in RunHandler src/ipc/chromium/src/base/message_loop.cc:318
    #34 0x7f330838bb2c in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:298
    #35 0x7f3312eb66e9 in XRE_InitChildProcess(int, char**, XREChildData const*) src/toolkit/xre/nsEmbedFunctions.cpp:770:34
    #36 0x4f2304 in content_process_main src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
    #37 0x4f2304 in main src/browser/app/nsBrowserApp.cpp:287
    #38 0x7f33269d282f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
    #39 0x421728 in _start (firefox+0x421728)
Flags: in-testsuite?
Assignee

Comment 1

9 months ago
Thanks!
Flags: needinfo?(emilio)
Assignee

Updated

9 months ago
Duplicate of this bug: 1487819
Crash Signature: [@ Servo_StyleSet_FlushStyleSheets] [@ geckoservo::glue::Servo_StyleSet_FlushStyleSheets]
Assignee

Updated

9 months ago
Flags: needinfo?(emilio)
Assignee

Updated

9 months ago
Assignee: nobody → emilio
Assignee

Comment 3

9 months ago
It can flush the parent document from MediaQueryList::RecomputeMatches, which
can destroy our pres context.

I think that flush is wrong btw, it should probably either flush layout so the
viewport size is correct, or just not flush (we should make MediaList::Matches
take a document now that we don't need a pres context to match media queries,
but that requires a bit more refactoring).
Tracking for 63 as it manifests mostly as startup crashes in 63.0b4
Assignee

Comment 5

9 months ago
I think conceptually we should flush layout instead of frames and do it
unconditionally, but everyone agrees on not doing that, and it can be really
slow, so will raise it to the CSSWG and get this spec'd.

See the test-case in bug 1458816, which tracks that.

I don't want to uplift this, but I think it's worth landing.
Comment on attachment 9008046 [details]
Bug 1490012 - FlushPendingMediaFeatureValuesChanged can kill the PresShell.

Xidorn Quan [:xidorn] UTC+10 has approved the revision.
Attachment #9008046 - Flags: review+
Comment on attachment 9008131 [details]
Don't flush on the parent document if we already have a pres context.

Xidorn Quan [:xidorn] UTC+10 has approved the revision.
Attachment #9008131 - Flags: review+

Comment 8

9 months ago
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc1f16ecff46
FlushPendingMediaFeatureValuesChanged can kill the PresShell. r=xidorn
https://hg.mozilla.org/integration/mozilla-inbound/rev/b499d1183c99
Don't flush on the parent document if we already have a pres context. r=xidorn

Comment 9

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cc1f16ecff46
https://hg.mozilla.org/mozilla-central/rev/b499d1183c99
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Xidorn, could your request an uplift to beta when you get a chance? We could uplift it after checking that the crashes are fixed on nightly.Thanks
Flags: needinfo?(xidorn+moz)
Assignee

Comment 11

9 months ago
Comment on attachment 9008046 [details]
Bug 1490012 - FlushPendingMediaFeatureValuesChanged can kill the PresShell.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1486536
[User impact if declined]: crashes
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: n/a
[Needs manual test from QE? If yes, steps to reproduce]: well, can check the crashtest.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not risky
[Why is the change risky/not risky?]: just a simple check for a condition that wasn't accounted for.
[String changes made/needed]: none
Flags: needinfo?(xidorn+moz)
Attachment #9008046 - Flags: approval-mozilla-beta?
Assignee

Comment 12

9 months ago
Note that the uplift request is only for the first patch.
Flags: in-testsuite? → in-testsuite+
Comment on attachment 9008046 [details]
Bug 1490012 - FlushPendingMediaFeatureValuesChanged can kill the PresShell.

Small patch fixing a crash, uplift approved for 63 beta 7,thanks.
Attachment #9008046 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.