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

Status: RESOLVED FIXED

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

Description

[Xidorn Quan [:xidorn] UTC+10]

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)

Comment 1

[Cameron McCormack (:heycam)]

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

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

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.

Comment 3

[Bobby Holley (:bholley)]

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

Comment 4

[Bobby Holley (:bholley)]

Attachment: Use a thread-safe URI comparison in CalcStyleDifference. v1

MozReview-Commit-ID: Fpb1guCxXUZ

Comment 5

[Cameron McCormack (:heycam)]

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adc82cb137bf
Use a thread-safe URI comparison in CalcStyleDifference. r=heycam

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/adc82cb137bf