stylo: Assertion failure: NS_IsMainThread(), at nsCSSValue.cpp:2823

RESOLVED FIXED in Firefox 55

Status

()

Core
CSS Parsing and Computation
P1
normal
RESOLVED FIXED
2 months ago
a month ago

People

(Reporter: xidorn, Assigned: bholley)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

I run into this issue when running layout/style/test/test_computed_style.html locally. Not sure why it doesn't crash on CI.

The stack is:
Assertion failure: NS_IsMainThread(), at /layout/style/nsCSSValue.cpp:2823
#01: mozilla::StyleShapeSource::operator== (\layout\style\nsstylestruct.h:2640)
#02: nsStyleSVGReset::CalcDifference (\layout\style\nsstylestruct.cpp:1194)
#03: nsStyleContext::CalcStyleDifferenceInternal<FakeStyleContext> (\layout\style\nsstylecontext.cpp:1142)
#04: nsStyleContext::CalcStyleDifference (\layout\style\nsstylecontext.cpp:1332)
#05: Gecko_CalcStyleDifference (\layout\style\servobindings.cpp:323)
#06: style::matching::PrivateMatchMethods::accumulate_damage<style::gecko::wrapper::GeckoElement> (\servo\components\style\matching.rs:684)
#07: style::matching::PrivateMatchMethods::cascade_primary_or_pseudo<style::gecko::wrapper::GeckoElement> (\servo\components\style\matching.rs:586)
#08: style::matching::MatchMethods::cascade_element<style::gecko::wrapper::GeckoElement> (\servo\components\style\matching.rs:1073)
#09: style::traversal::compute_style<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (\servo\components\style\traversal.rs:552)
#10: style::traversal::recalc_style_at<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (\servo\components\style\traversal.rs:433)
#11: style::gecko::traversal::{{impl}}::process_preorder (\servo\components\style\gecko\traversal.rs:46)
#12: style::parallel::traverse_nodes<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (\servo\components\style\parallel.rs:161)
#13: style::parallel::traverse_nodes<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (\servo\components\style\parallel.rs:162)
#14: style::parallel::traverse_nodes<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (\servo\components\style\parallel.rs:162)
#15: style::parallel::traverse_nodes<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (\servo\components\style\parallel.rs:162)
#16: style::parallel::traverse_dom::{{closure}}::{{closure}}<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> (\servo\components\style\parallel.rs:77)
#17: rayon::scope::{{impl}}::execute_job_closure::{{closure}}<closure> (\third_party\rust\rayon\src\scope\mod.rs:314)
#18: std::panic::{{impl}}::call_once<(),closure> (C:\bot\slave\stable-dist-rustc-win-msvc-64\build\src\libstd\panic.rs:296)
#19: std::panicking::try::do_call<std::panic::AssertUnwindSafe<closure>,()> (C:\bot\slave\stable-dist-rustc-win-msvc-64\build\src\libstd\panicking.rs:460)
#20: panic_abort::__rust_maybe_catch_panic (C:\bot\slave\stable-dist-rustc-win-msvc-64\build\src\libpanic_abort\lib.rs:42)
StyleShapeSource::operator== is calling URLValue::Equals, which indeed isn't safe to call OMT.  Instead, we should add a DefinitelyEquals method to it (which does calls URLValue::DefinitelyEquals) and call it from nsStyleSVGReset::CalcDifference (for mClipPath) and nsStyleDisplay::CalcDifference (for mShapeOutside).
FWIW, if I comment out that MOZ_ASSERT, it would hit the MOZ_ASSERT in URLValueData::GetURI().

If I comment out the latter MOZ_ASSERT, there is still something blocking the test from continuing... which seems to be some runtime type-safety insurance.
The reason this doesn't show up on CI is that all the tests run on single-core machines, and we end up doing a sequential traversal. See bug 1318187.

setting STYLO_THREADS=1 in the environment should work around this issue.

Would be good if somebody could land the fix
Priority: -- → P1
Assignee: nobody → bobbyholley
Created attachment 8848287 [details] [diff] [review]
Use a thread-safe URI comparison in CalcStyleDifference. v1

MozReview-Commit-ID: Fpb1guCxXUZ
Attachment #8848287 - Flags: review?(cam)
Comment on attachment 8848287 [details] [diff] [review]
Use a thread-safe URI comparison in CalcStyleDifference. v1

Review of attachment 8848287 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with that.

::: layout/style/nsStyleStruct.cpp
@@ +1190,5 @@
>  nsStyleSVGReset::CalcDifference(const nsStyleSVGReset& aNewData) const
>  {
>    nsChangeHint hint = nsChangeHint(0);
>  
> +  if (!mClipPath.DefinitelyEquals(aNewData.mClipPath)) {

Can you also call DefinitelyEquals in nsStyleDisplay for mShapeOutside?  (We don't have glue for that property yet, so it won't hit this assertion at the moment.)
Attachment #8848287 - Flags: review?(cam) → review+

Comment 6

a month ago
Pushed by bholley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/adc82cb137bf
Use a thread-safe URI comparison in CalcStyleDifference. r=heycam

Comment 7

a month ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/adc82cb137bf
Status: NEW → RESOLVED
Last Resolved: a month ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.