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

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: 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)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Thanks!

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1490012 - FlushPendingMediaFeatureValuesChanged can kill the PresShell.

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).

Comment 4

[Pascal Chevrel:pascalc]

Tracking for 63 as it manifests mostly as startup crashes in 63.0b4

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Don't flush on the parent document if we already have a pres context.

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 6

[Xidorn Quan [:xidorn] UTC+10]

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

Xidorn Quan [:xidorn] UTC+10 has approved the revision.

Comment 7

[Xidorn Quan [:xidorn] UTC+10]

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.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
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

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/cc1f16ecff46
https://hg.mozilla.org/mozilla-central/rev/b499d1183c99

Comment 10

[Pascal Chevrel:pascalc]

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

Comment 11

[Emilio Cobos Álvarez (:emilio)]

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

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Note that the uplift request is only for the first patch.

Comment 13

[Pascal Chevrel:pascalc]

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

Small patch fixing a crash, uplift approved for 63 beta 7,thanks.

Comment 14

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f09bf04f488c