Closed Bug 1403592 Opened 7 years ago Closed 7 years ago

stylo: panicked at 'already immutably borrowed'

Categories

(Core :: CSS Parsing and Computation, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: truber, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

(Keywords: assertion, testcase)

Attachments

(3 files)

Attached file testcase.html (deleted) —
The attached testcase panics in m-c rev 20170926-7d15bc419c6c thread '<unnamed>' panicked at 'already immutably borrowed', /builds/worker/workspace/build/src/third_party/rust/atomic_refcell/src/lib.rs:198 [159/1854] #0: mozalloc_abort, at memory/mozalloc/mozalloc_abort.cpp:33 #1: abort, at memory/mozalloc/mozalloc_abort.cpp:80 #2: panic_abort::__rust_start_panic, at src/libpanic_abort/lib.rs:61 #3: std::panicking::rust_panic, at src/libstd/panicking.rs:580 #4: std::panicking::rust_panic_with_hook, at src/libstd/panicking.rs:565 #5: std::panicking::begin_panic<collections::string::String>, at src/libstd/panicking.rs:511 #6: std::panicking::begin_panic_fmt, at src/libstd/panicking.rs:495 #7: atomic_refcell::AtomicBorrowRefMut::new, at obj-firefox/toolkit/library/gtest/rust/<panic macros>:8 #8: atomic_refcell::AtomicRefCell<style::gecko::data::PerDocumentStyleDataImpl>::borrow_mut<style::gecko::data::PerDocumentStyleDataImpl>, at third_party/rust/atomic_refcell/src/lib.rs:97 #9: style::gecko::data::PerDocumentStyleData::borrow_mut, at servo/components/style/gecko/data.rs:142 #10: geckoservo::glue::Servo_StyleSet_FlushStyleSheets, at servo/ports/geckolib/glue.rs:1050 #11: mozilla::ServoStyleSet::UpdateStylist, at layout/style/ServoStyleSet.cpp:1433 #12: mozilla::ServoStyleSet::AppendFontFaceRules, at layout/style/ServoStyleSet.cpp:1390 #13: nsIDocument::FlushUserFontSet, at layout/style/StyleSetHandleInlines.h:319 #14: nsIDocument::GetUserFontSet, at dom/base/nsDocument.cpp:13409 #15: nsRuleNode::GetMetricsFor, at layout/style/nsRuleNode.cpp:391 #16: Gecko_GetFontMetrics, at layout/style/ServoBindings.cpp:2485 #17: style::gecko::wrapper::{{impl}}::query, at servo/components/style/gecko/wrapper.rs:851 #18: style::values::specified::length::{{impl}}::reference_font_size_and_length::query_font_metrics, at servo/components/style/values/specified/length.rs:124 #19: style::values::specified::length::FontRelativeLength::reference_font_size_and_length, at servo/components/style/values/specified/length.rs:169 #20: style::values::specified::length::FontRelativeLength::to_computed_value, at servo/components/style/values/specified/length.rs:109 #21: style::values::computed::length::{{impl}}::to_computed_value, at servo/components/style/values/computed/length.rs:34 #22: style::values::computed::length::{{impl}}::to_computed_value, at servo/components/style/values/computed/length.rs:515 #23: style::properties::longhands::margin_left::cascade_property, at 4c5e399ab7a3687bb678171e1947c4c6e67e4e8b18b790cd09d14d9b64d11b65de9de42106af385d2ea0fd26d312a9bc18aa9566af2e1766945cfc4d52998e2e/toolkit/library/x86_64-unknown-linux-gnu/ debug/build/style-bccbf2b1c0916c0e/out/properties.rs:33599 #24: style::properties::apply_declarations<closure,core::iter::FlatMap<style::rule_tree::SelfAndAncestors, core::iter::FilterMap<core::iter::Rev<style::properties::declaration_block::DeclarationImportanceIterator>, closure>, closure>>, at 4c5e399ab7a3687bb678171e1947c4c6e67e4e8b18b790cd09d14d9b64d11b65de9de42106af385d2ea0fd26d312a9bc18aa9566af2e1766945cfc4d52998e2e/toolkit/library/x86_64-unknown-linux-gnu/debug/build/style-bccbf2b1c0916c0e/out/properties.rs:143127 #25: style::properties::cascade, at 4c5e399ab7a3687bb678171e1947c4c6e67e4e8b18b790cd09d14d9b64d11b65de9de42106af385d2ea0fd26d312a9bc18aa9566af2e1766945cfc4d52998e2e/toolkit/library/x86_64-unknown-linux-gnu/debug/build/style-bccbf2b1c0916c0 e/out/properties.rs:142737 #26: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_style<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:586 #27: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_style_and_visited<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:309 #28: style::style_resolver::{{impl}}::cascade_style_and_visited_with_default_parents::{{closure}}<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:279 #29: style::style_resolver::with_default_parent_styles<style::gecko::wrapper::GeckoElement,closure,style::style_resolver::ResolvedStyle>, at servo/components/style/style_resolver.rs:106 #30: style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_style_and_visited_with_default_parents<style::gecko::wrapper::GeckoElement>, at servo/components/style/style_resolver.rs:278 #31: geckoservo::glue::Servo_StyleSet_GetBaseComputedValuesForElement, at servo/ports/geckolib/glue.rs:745 #32: mozilla::ServoStyleSet::GetBaseContextForElement, at layout/style/ServoStyleSet.cpp:1237 #33: nsComputedDOMStyle::DoGetStyleContextNoFlush, at layout/style/nsComputedDOMStyle.cpp:717 #34: nsSMILCompositor::ComposeAttribute, at layout/style/nsComputedDOMStyle.h:121 #35: nsSMILAnimationController::DoSample, at dom/smil/nsSMILAnimationController.cpp:455 #36: nsSMILTimeContainer::Sample, at dom/smil/nsSMILTimeContainer.cpp:185 #37: nsSMILAnimationController::WillRefresh, at dom/smil/nsSMILAnimationController.cpp:167 #38: nsRefreshDriver::Tick, at layout/base/nsRefreshDriver.cpp:1886 #39: mozilla::RefreshDriverTimer::TickRefreshDrivers, at layout/base/nsRefreshDriver.cpp:307 #40: mozilla::RefreshDriverTimer::Tick, at layout/base/nsRefreshDriver.cpp:329 #41: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver, at layout/base/nsRefreshDriver.cpp:770 #42: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync, at layout/base/nsRefreshDriver.cpp:584 #43: mozilla::layout::VsyncChild::RecvNotify, at layout/ipc/VsyncChild.cpp:67 #44: mozilla::layout::PVsyncChild::OnMessageReceived, at 9a6479c28b0f57ad4e6d1aec1c5258678abc48225caeaa2a25f8a6893b83595687422ba915c00ef9e645c78dda14478aa8e2f891ae54d4764ccdba97f65bc47d/ipc/ipdl/PVsyncChild.cpp:155 #45: mozilla::ipc::MessageChannel::DispatchAsyncMessage, at ipc/glue/MessageChannel.cpp:2119 #46: mozilla::ipc::MessageChannel::DispatchMessage, at ipc/glue/MessageChannel.cpp:2049 #47: mozilla::ipc::MessageChannel::RunMessage, at ipc/glue/MessageChannel.cpp:1895 #48: mozilla::ipc::MessageChannel::MessageTask::Run, at ipc/glue/MessageChannel.cpp:1928 #49: nsThread::ProcessNextEvent, at xpcom/threads/nsThread.cpp:1039 #50: NS_ProcessNextEvent, at xpcom/threads/nsThreadUtils.cpp:522 #51: mozilla::ipc::MessagePump::Run, at ipc/glue/MessagePump.cpp:97 #52: MessageLoop::RunInternal, at ipc/chromium/src/base/message_loop.cc:326 #53: MessageLoop::Run, at ipc/chromium/src/base/message_loop.cc:319 #54: nsBaseAppShell::Run, at widget/nsBaseAppShell.cpp:158 #55: XRE_RunAppShell, at toolkit/xre/nsEmbedFunctions.cpp:880 #56: mozilla::ipc::MessagePumpForChildProcess::Run, at ipc/glue/MessagePump.cpp:269 #57: MessageLoop::RunInternal, at ipc/chromium/src/base/message_loop.cc:326 #58: MessageLoop::Run, at ipc/chromium/src/base/message_loop.cc:319 #59: XRE_InitChildProcess, at toolkit/xre/nsEmbedFunctions.cpp:705 #60: content_process_main, at ipc/contentproc/plugin-container.cpp:63 Changing the function to be called on 'window.onload' instead of 'requestIdleCallback' causes a different assertion: Assertion failure: ipcDoc, at /builds/worker/workspace/build/src/accessible/generic/Accessible.cpp:859 #0: mozilla::a11y::Accessible::HandleAccEvent, at mfbt/AlreadyAddRefed.h:106 #1: mozilla::a11y::AccessibleWrap::HandleAccEvent, at accessible/atk/AccessibleWrap.cpp:1187 #2: nsEventShell::FireEvent, at accessible/base/nsEventShell.cpp:45 #3: mozilla::a11y::DocAccessible::NotifyOfLoading, at accessible/generic/DocAccessible.cpp:1480 #4: mozilla::a11y::DocManager::OnStateChange, at accessible/base/DocManager.cpp:310 #5: nsDocLoader::DoFireOnStateChange, at uriloader/base/nsDocLoader.cpp:1320 #6: nsDocLoader::FireOnStateChange, at uriloader/base/nsDocLoader.cpp:1283 #7: nsDocLoader::doStartDocumentLoad, at uriloader/base/nsDocLoader.cpp:783 #8: nsDocLoader::OnStartRequest, at uriloader/base/nsDocLoader.cpp:456 #9: mozilla::net::nsLoadGroup::AddRequest, at netwerk/base/nsLoadGroup.cpp:510 #10: nsBaseChannel::AsyncOpen, at netwerk/base/nsBaseChannel.cpp:714 #11: nsBaseChannel::AsyncOpen2, at netwerk/base/nsBaseChannel.cpp:730 #12: nsURILoader::OpenURI, at uriloader/base/nsURILoader.cpp:840 #13: nsDocShell::DoChannelLoad, at docshell/base/nsDocShell.cpp:11715 #14: nsDocShell::DoURILoad, at docshell/base/nsDocShell.cpp:11520 #15: nsDocShell::InternalLoad, at docshell/base/nsDocShell.cpp:10857 #16: nsDocShell::LoadHistoryEntry, at docshell/base/nsDocShell.cpp:12901 #17: nsDocShell::Reload, at docshell/base/nsDocShell.cpp:5530 #18: mozilla::dom::Location::Reload, at dom/base/Location.cpp:875 #19: mozilla::dom::LocationBinding::reload, at dom/base/Location.h:56 #20: mozilla::dom::GenericBindingMethod, at dom/bindings/BindingUtils.cpp:3055 #21: js::CallJSNative, at js/src/jscntxtinlines.h:293
Flags: in-testsuite?
Ugh, this looks bad, we're flushing stylesheets from style resolution, wtf?
Assignee: nobody → emilio
Priority: -- → P2
Attachment #8912776 - Flags: review?(manishearth) → review+
Comment on attachment 8912772 [details] Bug 1403592: Never flush the user font set when getting font metrics from style resolution. https://reviewboard.mozilla.org/r/184082/#review189340
Attachment #8912772 - Flags: review?(manishearth) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 862aa3c61098 -d 77229cfe153a: rebasing 423085:862aa3c61098 "Bug 1403592: Never flush the user font set when getting font metrics from style resolution. r=manishearth" merging layout/style/ServoBindings.cpp merging layout/style/nsRuleNode.cpp rebasing 423086:4e742ed17303 "Bug 1403592: Crashtest. r=manishearth" (tip) merging layout/style/crashtests/crashtests.list warning: conflicts while merging layout/style/crashtests/crashtests.list! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/0a0cc4bb885a Never flush the user font set when getting font metrics from style resolution. r=Manishearth https://hg.mozilla.org/integration/autoland/rev/c050d8574203 Crashtest. r=Manishearth
Please either request uplift or set 57 status to wontfix.
Flags: needinfo?(emilio)
Comment on attachment 8912772 [details] Bug 1403592: Never flush the user font set when getting font metrics from style resolution. Approval Request Comment [Feature/Bug causing the regression]: stylo [User impact if declined]: crashes [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [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 one-liner that prevents some reentrancy in some edge cases. [String changes made/needed]: none
Flags: needinfo?(emilio)
Attachment #8912772 - Flags: approval-mozilla-beta?
Comment on attachment 8912772 [details] Bug 1403592: Never flush the user font set when getting font metrics from style resolution. Fix a crash, taking it. Should be in 57b5
Attachment #8912772 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Emilio Cobos Álvarez [:emilio] from comment #10) > [Is this code covered by automated tests?]: yes > [Has the fix been verified in Nightly?]: yes > [Needs manual test from QE? If yes, steps to reproduce]: no Setting qe-verify- based on Emilio's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: