Closed
Bug 1376130
Opened 7 years ago
Closed 7 years ago
stylo: frequent crashes in js::Allocate<T>: MOZ_RELEASE_ASSERT(!inUnsafeRegion) ([AutoAssertNoGC] possible GC in GC-unsafe region)
Categories
(Core :: CSS Parsing and Computation, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla56
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox54 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | fixed |
People
(Reporter: johnp, Assigned: xidorn)
References
(Blocks 1 open bug)
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
This bug was filed from the Socorro interface and is report bp-7d5a883a-447e-4c93-a2a1-d44fb0170624. ============================================================= Frame Module Signature Source 0 libxul.so js::Allocate<js::BaseShape, (js::AllowGC)1u> js/src/jscntxt.h:582 1 libxul.so js::BaseShape::getUnowned(JSContext*, js::StackBaseShape&) 2 libxul.so js::EmptyShape::getInitialShape(JSContext*, js::Class const*, js::TaggedProto, unsigned long, unsigned int) 3 libxul.so js::NewObjectWithGivenTaggedProto(JSContext*, js::Class const*, JS::Handle<js::TaggedProto>, js::gc::AllocKind, js::NewObjectKind, unsigned int) 4 libxul.so JS_NewObjectWithUniqueType(JSContext*, JSClass const*, JS::Handle<JSObject*>) 5 libxul.so XPCWrappedNativeProto::Init(nsIXPCScriptable*, bool) 6 libxul.so XPCWrappedNativeProto::GetNewOrUsed(XPCWrappedNativeScope*, nsIClassInfo*, nsIXPCScriptable*, bool) 7 libxul.so XPCWrappedNative::GetNewOrUsed(xpcObjectHelper&, XPCWrappedNativeScope*, XPCNativeInterface*, XPCWrappedNative**) 8 libxul.so XPCConvert::NativeInterface2JSObject(JS::MutableHandle<JS::Value>, nsIXPConnectJSObjectHolder**, xpcObjectHelper&, nsID const*, bool, nsresult*) 9 libxul.so nsXPConnect::WrapNative(JSContext*, JSObject*, nsISupports*, nsID const&, JSObject**) 10 libxul.so xpc_NewIDObject(JSContext*, JS::Handle<JSObject*>, nsID const&) 11 libxul.so nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject(JSContext*, JSObject*, nsID const&) 12 libxul.so nsXPCWrappedJS::GetNewOrUsed(JS::Handle<JSObject*>, nsID const&, nsXPCWrappedJS**) 13 libxul.so nsXPCWrappedJSClass::DelegatedQueryInterface(nsXPCWrappedJS*, nsID const&, void**) 14 libxul.so nsXPCWrappedJS::QueryInterface(nsID const&, void**) 15 libxul.so nsQueryReferent::operator() xpcom/base/nsWeakReference.cpp:148 16 libxul.so nsCOMPtr_base::assign_from_helper xpcom/base/nsCOMPtr.cpp:117 17 libxul.so nsObserverList::FillObserverArray(nsCOMArray<nsIObserver>&) 18 libxul.so nsObserverList::NotifyObservers(nsISupports*, char const*, char16_t const*) 19 libxul.so nsObserverService::NotifyObservers(nsISupports*, char const*, char16_t const*) 20 libxul.so nsDocument::OnPageHide(bool, mozilla::dom::EventTarget*) 21 libxul.so mozilla::image::SVGDocumentWrapper::DestroyViewer() 22 libxul.so mozilla::image::SVGDocumentWrapper::~SVGDocumentWrapper image/SVGDocumentWrapper.cpp:58 23 libxul.so libxul.so@0x117a597 24 libxul.so mozilla::image::VectorImage::~VectorImage mfbt/RefPtr.h:40 25 libxul.so libxul.so@0x117a77b 26 libxul.so imgRequest::~imgRequest() 27 libxul.so imgRequestProxy::~imgRequestProxy() 28 libxul.so imgRequestProxy::Release() 29 libxul.so PLDHashTable::Iterator::Remove xpcom/ds/PLDHashTable.cpp:661 30 libxul.so mozilla::css::ImageValue::~ImageValue layout/style/nsCSSValue.cpp:3088 31 libxul.so mozilla::css::ImageValue::~ImageValue layout/style/nsCSSValue.cpp:3090 32 libxul.so mozilla::css::URLValueData::Release layout/style/nsCSSValue.h:146 33 libxul.so core::ptr::drop_in_place<collections::vec::Vec<style::values::Either<style::values::None_, style::values::generics::image::Image<style::values::generics::image::Gradient<style::values::specified::image::LineDirection, style::values::specified::length::Length, style::values::specified::length::LengthOrPercentage, style::values::generics::position::Position<style::values::specified::position::PositionComponent<style::values::specified::position::X>, style::values::specified::position::PositionComponent<style::values::specified::position::Y>>, style::values::specified::color::RGBAColor>, style::values::generics::image::ImageRect<style::values::specified::NumberOrPercentage>>>>> servo/components/style/gecko_bindings/sugar/refptr.rs:250 34 libxul.so servo_arc::Arc<style::shared_lock::Locked<style::properties::declaration_block::PropertyDeclarationBlock>>::drop_slow<style::shared_lock::Locked<style::properties::declaration_block::PropertyDeclarationBlock>> /checkout/src/libcore/ptr.rs:66 35 libxul.so servo_arc::Arc<style::shared_lock::Locked<style::stylesheets::style_rule::StyleRule>>::drop_slow<style::shared_lock::Locked<style::stylesheets::style_rule::StyleRule>> servo/components/servo_arc/lib.rs:350 36 libxul.so servo_arc::Arc<style::shared_lock::Locked<style::stylesheets::rule_list::CssRules>>::drop_slow<style::shared_lock::Locked<style::stylesheets::rule_list::CssRules>> /checkout/src/libcore/ptr.rs:66 37 libxul.so servo_arc::Arc<style::stylesheets::stylesheet::Stylesheet>::drop_slow<style::stylesheets::stylesheet::Stylesheet> servo/components/servo_arc/lib.rs:350 38 libxul.so style::gecko::arc_types::Servo_StyleSheet_Release servo/components/servo_arc/lib.rs:350 39 libxul.so mozilla::ServoStyleSheetInner::~ServoStyleSheetInner layout/style/ServoArcTypeList.h:10 40 libxul.so mozilla::ServoStyleSheetInner::~ServoStyleSheetInner layout/style/ServoStyleSheet.cpp:56 41 libxul.so mozilla::StyleSheet::LastRelease() 42 libxul.so mozilla::StyleSheet::Release() 43 libxul.so nsCycleCollector::CollectWhite() 44 libxul.so nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) 45 libxul.so nsCycleCollector_collectSlice xpcom/base/nsCycleCollector.cpp:4201 46 libxul.so nsJSContext::RunCycleCollectorSlice(mozilla::TimeStamp) 47 libxul.so ICCRunnerFired(mozilla::TimeStamp, void*) 48 libxul.so CollectorRunner::Run dom/base/nsJSEnvironment.cpp:264 49 libxul.so nsThread::ProcessNextEvent(bool, bool*) 50 libxul.so NS_ProcessNextEvent(nsIThread*, bool) 51 libxul.so mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 52 libxul.so MessageLoop::Run() 53 libxul.so nsBaseAppShell::Run widget/nsBaseAppShell.cpp:156 54 libxul.so XRE_RunAppShell toolkit/xre/nsEmbedFunctions.cpp:896 55 libxul.so MessageLoop::Run() 56 libxul.so XRE_InitChildProcess toolkit/xre/nsEmbedFunctions.cpp:712 57 firefox content_process_main ipc/contentproc/plugin-container.cpp:64 58 firefox _init Ø 59 libc-2.25.so libc-2.25.so@0x204d9 60 firefox firefox@0x1182f 61 firefox firefox@0x1aeef 62 firefox firefox@0x1182f 63 firefox mozilla::ReadAheadLib(char const*) Ø 64 ld-2.25.so ld-2.25.so@0x1132f 65 firefox firefox@0x1aeef 66 firefox _start Another slightly different crash: This bug was filed from the Socorro interface and is report bp-3eafe5ed-849b-4725-9eb6-6c2a50170624. ============================================================= Frame Module Signature Source 0 libxul.so js::Allocate<JSObject, (js::AllowGC)1u> js/src/jscntxt.h:582 1 libxul.so js::NewObjectWithGivenTaggedProto(JSContext*, js::Class const*, JS::Handle<js::TaggedProto>, js::gc::AllocKind, js::NewObjectKind, unsigned int) 2 libxul.so JS_NewObjectWithGivenProto(JSContext*, JSClass const*, JS::Handle<JSObject*>) 3 libxul.so XPCWrappedNative::Init(nsIXPCScriptable*) 4 libxul.so XPCWrappedNative::GetNewOrUsed(xpcObjectHelper&, XPCWrappedNativeScope*, XPCNativeInterface*, XPCWrappedNative**) 5 libxul.so XPCConvert::NativeInterface2JSObject(JS::MutableHandle<JS::Value>, nsIXPConnectJSObjectHolder**, xpcObjectHelper&, nsID const*, bool, nsresult*) 6 libxul.so nsXPConnect::WrapNative(JSContext*, JSObject*, nsISupports*, nsID const&, JSObject**) 7 libxul.so xpc_NewIDObject(JSContext*, JS::Handle<JSObject*>, nsID const&) 8 libxul.so nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject(JSContext*, JSObject*, nsID const&) 9 libxul.so nsXPCWrappedJS::GetNewOrUsed(JS::Handle<JSObject*>, nsID const&, nsXPCWrappedJS**) 10 libxul.so nsXPCWrappedJSClass::DelegatedQueryInterface(nsXPCWrappedJS*, nsID const&, void**) 11 libxul.so nsXPCWrappedJS::QueryInterface(nsID const&, void**) 12 libxul.so nsQueryReferent::operator() xpcom/base/nsWeakReference.cpp:148 13 libxul.so nsCOMPtr_base::assign_from_helper xpcom/base/nsCOMPtr.cpp:117 14 libxul.so nsObserverList::FillObserverArray(nsCOMArray<nsIObserver>&) 15 libxul.so nsObserverList::NotifyObservers(nsISupports*, char const*, char16_t const*) 16 libxul.so nsObserverService::NotifyObservers(nsISupports*, char const*, char16_t const*) 17 libxul.so nsDocument::OnPageHide(bool, mozilla::dom::EventTarget*) 18 libxul.so mozilla::image::SVGDocumentWrapper::DestroyViewer() 19 libxul.so mozilla::image::SVGDocumentWrapper::~SVGDocumentWrapper image/SVGDocumentWrapper.cpp:58 20 libxul.so libxul.so@0x117a597 21 libxul.so mozilla::image::VectorImage::~VectorImage mfbt/RefPtr.h:40 22 libxul.so libxul.so@0x117a77b 23 libxul.so imgRequest::~imgRequest() 24 libxul.so imgRequestProxy::~imgRequestProxy() 25 libxul.so imgRequestProxy::Release() 26 libxul.so PLDHashTable::Iterator::Remove xpcom/ds/PLDHashTable.cpp:661 27 libxul.so mozilla::css::ImageValue::~ImageValue layout/style/nsCSSValue.cpp:3088 28 libxul.so mozilla::css::ImageValue::~ImageValue layout/style/nsCSSValue.cpp:3090 29 libxul.so mozilla::css::URLValueData::Release layout/style/nsCSSValue.h:146 30 libxul.so core::ptr::drop_in_place<collections::vec::Vec<style::values::Either<style::values::None_, style::values::generics::image::Image<style::values::generics::image::Gradient<style::values::specified::image::LineDirection, style::values::specified::length::Length, style::values::specified::length::LengthOrPercentage, style::values::generics::position::Position<style::values::specified::position::PositionComponent<style::values::specified::position::X>, style::values::specified::position::PositionComponent<style::values::specified::position::Y>>, style::values::specified::color::RGBAColor>, style::values::generics::image::ImageRect<style::values::specified::NumberOrPercentage>>>>> servo/components/style/gecko_bindings/sugar/refptr.rs:250 31 libxul.so servo_arc::Arc<style::shared_lock::Locked<style::properties::declaration_block::PropertyDeclarationBlock>>::drop_slow<style::shared_lock::Locked<style::properties::declaration_block::PropertyDeclarationBlock>> /checkout/src/libcore/ptr.rs:66 32 libxul.so servo_arc::Arc<style::shared_lock::Locked<style::stylesheets::style_rule::StyleRule>>::drop_slow<style::shared_lock::Locked<style::stylesheets::style_rule::StyleRule>> servo/components/servo_arc/lib.rs:350 33 libxul.so servo_arc::Arc<style::shared_lock::Locked<style::stylesheets::rule_list::CssRules>>::drop_slow<style::shared_lock::Locked<style::stylesheets::rule_list::CssRules>> /checkout/src/libcore/ptr.rs:66 34 libxul.so servo_arc::Arc<style::stylesheets::stylesheet::Stylesheet>::drop_slow<style::stylesheets::stylesheet::Stylesheet> servo/components/servo_arc/lib.rs:350 35 libxul.so style::gecko::arc_types::Servo_StyleSheet_Release servo/components/servo_arc/lib.rs:350 36 libxul.so mozilla::ServoStyleSheetInner::~ServoStyleSheetInner layout/style/ServoArcTypeList.h:10 37 libxul.so mozilla::ServoStyleSheetInner::~ServoStyleSheetInner layout/style/ServoStyleSheet.cpp:56 38 libxul.so mozilla::StyleSheet::LastRelease() 39 libxul.so mozilla::StyleSheet::Release() 40 libxul.so nsCycleCollector::CollectWhite() 41 libxul.so nsCycleCollector::Collect(ccType, js::SliceBudget&, nsICycleCollectorListener*, bool) 42 libxul.so nsCycleCollector_collectSlice xpcom/base/nsCycleCollector.cpp:4201 43 libxul.so nsJSContext::RunCycleCollectorSlice(mozilla::TimeStamp) 44 libxul.so ICCRunnerFired(mozilla::TimeStamp, void*) 45 libxul.so CollectorRunner::Run dom/base/nsJSEnvironment.cpp:264 46 libxul.so CollectorRunner::TimedOut dom/base/nsJSEnvironment.cpp:281 47 libxul.so nsTimerImpl::Fire(int) 48 libxul.so nsTimerEvent::Run() 49 libxul.so nsThread::ProcessNextEvent(bool, bool*) 50 libxul.so NS_ProcessNextEvent(nsIThread*, bool) 51 libxul.so mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 52 libxul.so MessageLoop::Run() 53 libxul.so nsBaseAppShell::Run widget/nsBaseAppShell.cpp:156 54 libxul.so XRE_RunAppShell toolkit/xre/nsEmbedFunctions.cpp:896 55 libxul.so MessageLoop::Run() 56 libxul.so XRE_InitChildProcess toolkit/xre/nsEmbedFunctions.cpp:712 57 firefox content_process_main ipc/contentproc/plugin-container.cpp:64 58 firefox _init Ø 59 libc-2.25.so libc-2.25.so@0x204d9 60 firefox firefox@0x1182f 61 firefox firefox@0x1aeef Ø 62 libfreetype.so.6.13.0 libfreetype.so.6.13.0@0x255fff 63 firefox firefox@0x1182f 64 firefox mozilla::ReadAheadLib(char const*) Ø 65 ld-2.25.so ld-2.25.so@0x1132f 66 firefox firefox@0x1aeef 67 firefox _start I don't have any specific STR, the tab crash just happens seemingly randomly when browsing e.g. YouTube and twitter (50+ times today) since setting layout.css.servo.enabled=true. Disabling the flag makes the crashes go away. (except for some cases where I think the tabs were loaded when stylo was still enabled - if that makes sense)
Reporter | ||
Comment 1•7 years ago
|
||
After some investigating it seems to be that the crash only happens when LastPass (4.1.54a) is enabled in my profile.
Reporter | ||
Comment 2•7 years ago
|
||
STR for fresh profile: * Install LastPass 4.1.54a: https://addons.mozilla.org/en-US/firefox/addon/lastpass-password-manager/versions/beta * Visit https://www.youtube.com * Browse a bit -> crash (tab crash or whole browser crash) * Result: bp-3d26fb7a-ba7e-4bd1-ad3f-ed9d70170624
Reporter | ||
Updated•7 years ago
|
Has STR: --- → yes
Reporter | ||
Comment 3•7 years ago
|
||
(STR addendum: I have only been able to reproduce the bug when logged into my LastPass account)
Comment 4•7 years ago
|
||
I came here from this crash report and I do not use Lastpass https://crash-stats.mozilla.com/report/index/25cb18a6-daed-4963-ab7b-ad4c90170625
Comment 5•7 years ago
|
||
The signature 'js::Allocate<T>' has reappeared with buildid 20170623100152. There are 181 crashes. :jonco, could you investigate ?
Component: CSS Parsing and Computation → JavaScript: GC
Flags: needinfo?(jcoppeard)
Comment 6•7 years ago
|
||
(In reply to Calixte Denizet (:calixte) from comment #5) > The signature 'js::Allocate<T>' has reappeared with buildid 20170623100152. > There are 181 crashes. > :jonco, could you investigate ? This is a Stylo issue. Stylo is somehow causing SVG to be destroyed, which is notifying observers, which is causing JS to run. I'm not sure why Stylo in particular is triggering this.
Component: JavaScript: GC → CSS Parsing and Computation
Flags: needinfo?(jcoppeard)
Comment 7•7 years ago
|
||
We have an assertion that JS can't be called during CollectWhite. I'm not sure how critical that is to anything, but it would be unfortunate to regress this nice property incidentally.
Reporter | ||
Updated•7 years ago
|
Blocks: stylo-site-issues
Assignee | ||
Comment 8•7 years ago
|
||
Looking at the stack, it is unclear to me why this only happens in Stylo either. It doesn't seem to me there is anything in Gecko stopping ImageValue from being destroyed when StyleSheet gets destroyed.
Comment 9•7 years ago
|
||
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #8) > Looking at the stack, it is unclear to me why this only happens in Stylo > either. It doesn't seem to me there is anything in Gecko stopping ImageValue > from being destroyed when StyleSheet gets destroyed. This is happening on the last release of the style sheet, not when the style sheet is destroyed. Because the style sheet is cycle collected, the actual dtor call will happen later.
Assignee | ||
Comment 10•7 years ago
|
||
This is a recent change (bug 1373484) to both Gecko and Stylo. And yeah, it is on the last release. But on the last release, there doesn't seem to be anything in Gecko to stop that as well, so in theory this should happen to Gecko as well. Is CollectWhite called only when the object is cycle collected? IIUC, if the object is not cycle referenced, LastRelease should be called immediately at the last Release call.
Assignee | ||
Comment 11•7 years ago
|
||
I'm still not able to reproduce this crash with a testcase. What I did is: * add an observer in script for content-page-hidden * load a document with SVG image referenced in a stylesheet from a <style> * remove the <style> element I can see that observer is triggered for the SVG document, but it doesn't crash. One thing I noticed is that, the observer is triggered for the SVG document significantly later than the document which references it, and neither CC nor GC can trigger that, which probably means we are keeping it in some cache. If we always keep it in some cache, presumably it shouldn't be released from stylesheet release at all I suppose.
Updated•7 years ago
|
Assignee: nobody → xidorn+moz
Priority: -- → P1
Assignee | ||
Comment 12•7 years ago
|
||
OK, so I have a STR now. Thanks for dholbert's help. 1. add a script observer to content-page-hidden. I added one in browser.js. 2. open a page which has SVG referenced from stylesheet, and the image is actually used in the document. 3. press Ctrl-Shift-Del and clear the cache 4. then close the page And why it is different than Gecko is because, in Gecko, StyleRule holds the image value, and StyleRule is somehow always destroyed by SnowWhiteKiller, while in Stylo, StyleSheet holds the values via Servo side Stylesheet somehow, and that is destroyed by some other thing, which can trigger this crash.
Assignee | ||
Comment 13•7 years ago
|
||
So there are two ideas how we can fix this: 1. Don't notify that observer (and probably many other things) for SVG-as-image (which can be checked via nsIDocument::mIsBeingUsedAsImage) 2. Put Servo_StyleSheet_Release into a runnable and make it run asynchonrously I guess actually it's worth doing both. For the first one, it doesn't seem to make much sense to trigger lots of such things for a document which is used as image. That could be annoying for their observers as well, and they can potentially trigger similar crash in other places. For the second, there is nothing guarantees that we wouldn't have other places trigger this kind of issues.
Comment hidden (mozreview-request) |
Comment 15•7 years ago
|
||
Why do we want the patch? Why we want to special case some type of documents? Feels odd that we dispatch pageshow/hide events but don't fire the notification. I don't like that inconsistency.
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8882627 [details] Bug 1376130 - Don't notify page-{shown,hidden} observers or dispatch pageshow/pagehide events when the document is an SVG used as image. https://reviewboard.mozilla.org/r/153708/#review158882 So, this feels like a hack, which makes pagehide/show inconsistent. Could we do the other approach?
Attachment #8882627 -
Flags: review?(bugs) → review-
Assignee | ||
Comment 17•7 years ago
|
||
In general, I don't think anyone should be able to access document inside SVG-as-image. If nothing is able to access the document, nothing can hook an event handler on it. I wonder whether there is a general way we can fully avoid the document to be accessed.
Comment 18•7 years ago
|
||
Don't we fire a notification when a document is created or document element added to it.
Assignee | ||
Comment 19•7 years ago
|
||
It seems this is not the first time we hit OnPageHide for SVG-as-image document in some weird place. See also bug 1309960 which happens when we tried doing GC and painting in parallel. This time, we hit this inside CC. I guess this case can happen for Gecko in some cases as well, just more tricky. Actually, the current design of Gecko isn't meant to protect from this case at all. It just happens to not crash. (It is also released from CC according to the stack, but somehow not in a path which GC is forbidden.) Basically, places handling image don't usually take script into account, because in general that doesn't happen. That only happens when it is an SVG-as-image, and some addon tries to add observer of content-page-hidden. It is also tricky to write test for this kind of crash, because the ownership of related objects is kind of complicated. That means we are usually aware of this kind of crash only after it actually hits some people, which doesn't sound very helpful. We really should avoid notifying that. If you are concerned about the inconsistency between the notification and the pageshow/pagehide event, I can move them into the if-block as well. Are you fine with that?
See Also: → 1309960
Comment 20•7 years ago
|
||
That sounds better yes.
Comment hidden (mozreview-request) |
Comment 22•7 years ago
|
||
mozreview-review |
Comment on attachment 8882627 [details] Bug 1376130 - Don't notify page-{shown,hidden} observers or dispatch pageshow/pagehide events when the document is an SVG used as image. https://reviewboard.mozilla.org/r/153708/#review159090
Attachment #8882627 -
Flags: review?(bugs) → review+
Comment 23•7 years ago
|
||
Pushed by xquan@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/00be20066a63 Don't notify page-{shown,hidden} observers or dispatch pageshow/pagehide events when the document is an SVG used as image. r=smaug
Comment 24•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/00be20066a63
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Updated•7 years ago
|
status-firefox54:
--- → unaffected
status-firefox55:
--- → unaffected
status-firefox-esr52:
--- → unaffected
Comment 26•6 months ago
|
||
Copying crash signatures from duplicate bugs.
Crash Signature: [@ js::Allocate<T>] → [@ js::Allocate<T>]
[@ js::BarrierMethods<T>::exposeToJS]
You need to log in
before you can comment on or make changes to this bug.
Description
•