#0 0x51b557 in MOZ_CrashOOL /src/mfbt/Assertions.cpp:33:3 #1 0x7f3653bce2f2 in nsAutoOwningThread::AssertCurrentThreadOwnsMe(char const*) const /src/xpcom/base/nsISupportsImpl.cpp:43:5 #2 0x7f3656264ac8 in AssertOwnership<29> /src/obj-firefox/dist/include/nsISupportsImpl.h:69:5 #3 0x7f3656264ac8 in Release /src/image/ImageWrapper.cpp:128 #4 0x7f3656264ac8 in mozilla::image::ClippedImage::Release() /src/image/ClippedImage.cpp:213 #5 0x7f3653c14427 in ReleaseObjects /src/xpcom/ds/nsCOMArray.cpp:272:5 #6 0x7f3653c14427 in nsCOMArray_base::Clear() /src/xpcom/ds/nsCOMArray.cpp:281 #7 0x7f3653c28a4e in nsCOMArray_base::~nsCOMArray_base() /src/xpcom/ds/nsCOMArray.cpp:52:3 #8 0x7f365a8472a6 in ~CachedBorderImageData /src/obj-firefox/dist/include/nsStyleStruct.h:399:8 #9 0x7f365a8472a6 in operator() /src/obj-firefox/dist/include/mozilla/UniquePtr.h:528 #10 0x7f365a8472a6 in reset /src/obj-firefox/dist/include/mozilla/UniquePtr.h:343 #11 0x7f365a8472a6 in ~UniquePtr /src/obj-firefox/dist/include/mozilla/UniquePtr.h:288 #12 0x7f365a8472a6 in nsStyleImage::~nsStyleImage() /src/layout/style/nsStyleStruct.cpp:2250 #13 0x7f365a836d70 in nsStyleBorder::~nsStyleBorder() /src/layout/style/nsStyleStruct.cpp:408:1 #14 0x7f3660299571 in style::gecko_properties::{{impl}}::drop /src/obj-firefox/toolkit/library/x86_64-unknown-linux-gnu/release/build/style-dc7c941fc56784c9/out/gecko_properties.rs:22064 #15 0x7f3660299571 in core::ptr::drop_in_place<style::gecko_bindings::structs::root::mozilla::GeckoBorder> /checkout/src/libcore/ptr.rs:60 #16 0x7f3660299571 in core::ptr::drop_in_place<servo_arc::ArcInner<style::gecko_bindings::structs::root::mozilla::GeckoBorder>> /checkout/src/libcore/ptr.rs:60 #17 0x7f3660299571 in core::ptr::drop_in_place<alloc::boxed::Box<servo_arc::ArcInner<style::gecko_bindings::structs::root::mozilla::GeckoBorder>>> /checkout/src/libcore/ptr.rs:60 #18 0x7f3660299571 in _$LT$servo_arc..Arc$LT$T$GT$$GT$::drop_slow::h0fb47fbba26bb271 /src/servo/components/servo_arc/lib.rs:245 #19 0x7f3660298f5a in servo_arc::{{impl}}::drop<style::gecko_bindings::structs::root::mozilla::GeckoBorder> /src/servo/components/servo_arc/lib.rs:379 #20 0x7f3660298f5a in core::ptr::drop_in_place<servo_arc::Arc<style::gecko_bindings::structs::root::mozilla::GeckoBorder>> /checkout/src/libcore/ptr.rs:60 #21 0x7f3660298f5a in servo_arc::{{impl}}::drop<style::gecko_bindings::structs::root::mozilla::GeckoBorder> /src/servo/components/servo_arc/lib.rs:775 #22 0x7f3660298f5a in core::ptr::drop_in_place<servo_arc::RawOffsetArc<style::gecko_bindings::structs::root::mozilla::GeckoBorder>> /checkout/src/libcore/ptr.rs:60 #23 0x7f3660298f5a in core::ptr::drop_in_place<style::gecko_bindings::structs::root::ServoComputedData> /checkout/src/libcore/ptr.rs:60 #24 0x7f3660298f5a in core::ptr::drop_in_place<style::gecko_bindings::structs::root::mozilla::ServoStyleContext> /checkout/src/libcore/ptr.rs:60 #25 0x7f3660298f5a in core::ptr::drop_in_place<style::gecko_properties::ComputedValues> /checkout/src/libcore/ptr.rs:60 #26 0x7f3660298f5a in core::ptr::drop_in_place<servo_arc::ArcInner<style::gecko_properties::ComputedValues>> /checkout/src/libcore/ptr.rs:60 #27 0x7f3660298f5a in core::ptr::drop_in_place<alloc::boxed::Box<servo_arc::ArcInner<style::gecko_properties::ComputedValues>>> /checkout/src/libcore/ptr.rs:60 #28 0x7f3660298f5a in _$LT$servo_arc..Arc$LT$T$GT$$GT$::drop_slow::h0a5fa88d824d76d8 /src/servo/components/servo_arc/lib.rs:245 #29 0x7f366028e238 in servo_arc::{{impl}}::drop<style::gecko_properties::ComputedValues> /src/servo/components/servo_arc/lib.rs:379 #30 0x7f366028e238 in core::ptr::drop_in_place<servo_arc::Arc<style::gecko_properties::ComputedValues>> /checkout/src/libcore/ptr.rs:60 #31 0x7f366028e238 in style::matching::MatchMethods::finish_restyle<style::gecko::wrapper::GeckoElement> /src/servo/components/style/matching.rs:667 #32 0x7f366028e238 in style::traversal::compute_style::hf61de7b9017a79f1 /src/servo/components/style/traversal.rs:726 #33 0x7f3660296082 in style::traversal::recalc_style_at<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly,closure> /src/servo/components/style/traversal.rs:477 #34 0x7f3660296082 in style::gecko::traversal::{{impl}}::process_preorder<closure> /src/servo/components/style/gecko/traversal.rs:37 #35 0x7f3660296082 in style::parallel::top_down_dom<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly> /src/servo/components/style/parallel.rs:188 #36 0x7f3660296082 in style::parallel::traverse_nodes::{{closure}}<style::gecko::wrapper::GeckoElement,style::gecko::traversal::RecalcStyleOnly,collections::vec_deque::Drain<style::dom::SendNode<style::gecko::wrapper::GeckoNode>>> /src/servo/components/style/parallel.rs:282 #37 0x7f3660296082 in rayon_core::scope::{{impl}}::execute_job_closure::{{closure}}<closure,()> /src/third_party/rust/rayon-core/src/scope/mod.rs:354 #38 0x7f3660296082 in std::panic::{{impl}}::call_once<(),closure> /checkout/src/libstd/panic.rs:296 #39 0x7f3660296082 in std::panicking::try::do_call<std::panic::AssertUnwindSafe<closure>,()> /checkout/src/libstd/panicking.rs:454 #40 0x7f3660296082 in panic_abort::__rust_maybe_catch_panic /checkout/src/libpanic_abort/lib.rs:40 #41 0x7f3660296082 in std::panicking::try<(),std::panic::AssertUnwindSafe<closure>> /checkout/src/libstd/panicking.rs:433 #42 0x7f3660296082 in std::panic::catch_unwind<std::panic::AssertUnwindSafe<closure>,()> /checkout/src/libstd/panic.rs:361 #43 0x7f3660296082 in rayon_core::unwind::halt_unwinding<closure,()> /src/third_party/rust/rayon-core/src/unwind.rs:19 #44 0x7f3660296082 in rayon_core::scope::{{impl}}::execute_job_closure<closure,()> /src/third_party/rust/rayon-core/src/scope/mod.rs:354 #45 0x7f3660296082 in rayon_core::scope::{{impl}}::execute_job<closure> /src/third_party/rust/rayon-core/src/scope/mod.rs:343 #46 0x7f3660296082 in rayon_core::scope::{{impl}}::spawn::{{closure}}<closure> /src/third_party/rust/rayon-core/src/scope/mod.rs:274 #47 0x7f3660296082 in _$LT$rayon_core..job..HeapJob$LT$BODY$GT$$u20$as$u20$rayon_core..job..Job$GT$::execute::he7e1344a87a39d18 /src/third_party/rust/rayon-core/src/job.rs:142 #48 0x7f36607aa729 in rayon_core::job::{{impl}}::execute /src/third_party/rust/rayon-core/src/job.rs:55 #49 0x7f36607aa729 in rayon_core::registry::{{impl}}::execute /src/third_party/rust/rayon-core/src/registry.rs:476 #50 0x7f36607aa729 in rayon_core::registry::{{impl}}::wait_until_cold<rayon_core::latch::CountLatch> /src/third_party/rust/rayon-core/src/registry.rs:460 #51 0x7f36607aa729 in rayon_core::registry::WorkerThread::wait_until::he55fc0dee82c0e2c /src/third_party/rust/rayon-core/src/registry.rs:436 #52 0x7f36607aa2e9 in rayon_core::registry::main_loop /src/third_party/rust/rayon-core/src/registry.rs:559 #53 0x7f36607aa2e9 in rayon_core::registry::{{impl}}::new::{{closure}} /src/third_party/rust/rayon-core/src/registry.rs:145 #54 0x7f36607aa2e9 in std::sys_common::backtrace::__rust_begin_short_backtrace::h5fc46c8445d7f702 /checkout/src/libstd/sys_common/backtrace.rs:136 #55 0x7f36607aa0a9 in std::thread::{{impl}}::spawn::{{closure}}::{{closure}}<closure,()> /checkout/src/libstd/thread/mod.rs:364 #56 0x7f36607aa0a9 in std::panic::{{impl}}::call_once<(),closure> /checkout/src/libstd/panic.rs:296 #57 0x7f36607aa0a9 in std::panicking::try::do_call<std::panic::AssertUnwindSafe<closure>,()> /checkout/src/libstd/panicking.rs:454 #58 0x7f36607aa0a9 in panic_abort::__rust_maybe_catch_panic /checkout/src/libpanic_abort/lib.rs:40 #59 0x7f36607aa0a9 in std::panicking::try<(),std::panic::AssertUnwindSafe<closure>> /checkout/src/libstd/panicking.rs:433 #60 0x7f36607aa0a9 in std::panic::catch_unwind<std::panic::AssertUnwindSafe<closure>,()> /checkout/src/libstd/panic.rs:361 #61 0x7f36607aa0a9 in std::thread::{{impl}}::spawn::{{closure}}<closure,()> /checkout/src/libstd/thread/mod.rs:363 #62 0x7f36607aa0a9 in _$LT$F$u20$as$u20$alloc..boxed..FnBox$LT$A$GT$$GT$::call_box::h8c2f63f0700774f4 /checkout/src/liballoc/boxed.rs:648 #63 0x7f36607ffcf3 in alloc::boxed::{{impl}}::call_once<(),()> /checkout/src/liballoc/boxed.rs:658 #64 0x7f36607ffcf3 in std::sys_common::thread::start_thread /checkout/src/libstd/sys_common/thread.rs:21 #65 0x7f36607ffcf3 in std::sys::imp::thread::Thread::new::thread_start::h227b2afaa9316a8d /checkout/src/libstd/sys/unix/thread.rs:84 #66 0x7f36727646b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9) #67 0x7f36717ed3dc in clone /build/glibc-bfm8X4/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

I couldn't reproduce, but the stack looks more like stylo is releasing objects on the wrong thread?

I can reproduce this issue with the latest build in my local Mac. The attached file was the back trace I hit. I will take some time to get more information.

From looked into this, ImageWrapper was constructed in main thread but was released in stylo thread. ImageWrapper isn't a thread-safe object so nsAutoOwningThread was detected for this. Maybe we should. 1. Find out why the release flow run in the wrong thread. Or. 2. Post to main-thread to destruct ImageWrapper.

(In reply to Vincent Liu[:vliu] from comment #3) > From looked into this, ImageWrapper was constructed in main thread but was > released in stylo thread. ImageWrapper isn't a thread-safe object so > nsAutoOwningThread was detected for this. Maybe we should. > > 1. Find out why the release flow run in the wrong thread. Or. > 2. Post to main-thread to destruct ImageWrapper. I have to correct the previous information that the construction/releasing were both in style thread. Sorry about that.

RasterImage (and VectorImage, I think?) needs to be freed on the main thread. If it is possible that the ImageWrapper contains the last reference to the underlying RasterImage/VectorImage, it will also need to free its reference on the main thread. This is easily solved with a proxy release in ~ImageWrapper, but something to keep in mind here.

In general I think we defer image resolution for computed values to the main thread. Dropping _mostly_ happens on the main thread (because the style is held alive by the frame, which is updated during the post-traversal), but there are situations where we might not have a frame. So we probably want to check IsInServoTraversal() in the nsStyleImage destructor, and use NS_ReleaseOnMainThread if so.

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #6) > In general I think we defer image resolution for computed values to the main > thread. Dropping _mostly_ happens on the main thread (because the style is > held alive by the frame, which is updated during the post-traversal), but > there are situations where we might not have a frame. > > So we probably want to check IsInServoTraversal() in the nsStyleImage > destructor, and use NS_ReleaseOnMainThread if so. Thanks for the suggestion. From the current understanding, if I move the UniquePtr mCachedBIData into main-thread to release called in the nsStyleImage destructor, it should be fixed. I will have a patch soon to fix it.

(In reply to Vincent Liu[:vliu] from comment #7) > (In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #6) > > In general I think we defer image resolution for computed values to the main > > thread. Dropping _mostly_ happens on the main thread (because the style is > > held alive by the frame, which is updated during the post-traversal), but > > there are situations where we might not have a frame. > > > > So we probably want to check IsInServoTraversal() in the nsStyleImage > > destructor, and use NS_ReleaseOnMainThread if so. > > Thanks for the suggestion. From the current understanding, if I move the > UniquePtr mCachedBIData into main-thread to release called in the > nsStyleImage destructor, it should be fixed. I will have a patch soon to fix > it. Wouldn't it be easier to just have ~CachedBorderImageData invoke PurgeCachedImages, which already handles the OMT case?

(Thanks for taking this btw - to be clear, we need this fix in before the branch on Thursday)

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #8) > > Wouldn't it be easier to just have ~CachedBorderImageData invoke > PurgeCachedImages, which already handles the OMT case? Thanks for the good suggestion. Could you please have a review?

try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d469f6a594af3f78c73c10eb68639d75baa8427a

Pushed by vliu@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5fa2260faa05 Invoke PurgeCachedImages to release object in main thread while ServoTraversal. r=bholley

Can we land a test for this?

We should add test_case.html as 1399006.html to layout/style/crashtests/crashtests.list

Hi bobby, Can you please help to review the patch for crash test? Thanks

Comment on attachment 8911034 [details] [diff] [review] 0001-Bug-1399006-Add-crashtest.-r-bholley.patch Review of attachment 8911034 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/style/crashtests/crashtests.list @@ +218,5 @@ > load 1397091.html > load 1398479.html > load 1398581.html > load 1400035.html > load 1399546.html Nit: while you are here, could you swap 1399546.html and 1400035.html and added your test accordingly so that they are in order?

(In reply to Ting-Yu Lin [:TYLin] (UTC+8) from comment #17) > Comment on attachment 8911034 [details] [diff] [review] > Nit: while you are here, could you swap 1399546.html and 1400035.html and > added your test accordingly so that they are in order? Thanks for the suggestions. The modified patch was attached with nit and r+ carried.

try result for the crash test case. try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a3f432675b887802afa1a1fe108da40490b158f&selectedJob=132638700

Pushed by vliu@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f65c1859b21e Add crashtest. r=TYLin