Closed Bug 1490012 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 + fixed
firefox64 --- fixed

People

(Reporter: tsmith, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(3 files)

Attached 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?
Thanks!
Flags: needinfo?(emilio)
Duplicate of this bug: 1487819
Crash Signature: [@ Servo_StyleSet_FlushStyleSheets] [@ geckoservo::glue::Servo_StyleSet_FlushStyleSheets]
Flags: needinfo?(emilio)
Assignee: nobody → emilio
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
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+
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
https://hg.mozilla.org/mozilla-central/rev/cc1f16ecff46
https://hg.mozilla.org/mozilla-central/rev/b499d1183c99
Status: NEW → RESOLVED
Closed: 2 years 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)
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?
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.