Open Bug 1656851 Opened 4 years ago Updated 1 year ago

Crash in [@ mozalloc_abort | style::custom_properties::CustomPropertiesBuilder::cascade]

Categories

(Core :: Layout, defect, P3)

All
Windows
defect

Tracking

()

People

(Reporter: gsvelto, Unassigned)

References

Details

(Keywords: crash, Whiteboard: qa-not-actionable)

Crash Data

This bug is for crash report bp-ff32bfc5-bcd6-4b94-9068-9e8d70200801.

Top 10 frames of crashing thread:

0 mozglue.dll mozalloc_abort memory/mozalloc/mozalloc_abort.cpp:33
1 mozglue.dll mozalloc_handle_oom memory/mozalloc/mozalloc_oom.cpp:51
2 xul.dll gkrust_shared::oom_hook::hook toolkit/library/rust/shared/lib.rs:127
3 xul.dll std::alloc::rust_oom ../4fb7144ed159f94491249e86d5bbd033b5d60550//src/libstd/alloc.rs:240
4 xul.dll style::custom_properties::CustomPropertiesBuilder::cascade servo/components/style/custom_properties.rs:563
5 xul.dll style::properties::cascade::cascade_rules<style::gecko::wrapper::GeckoElement> servo/components/style/properties/cascade.rs:210
6 xul.dll style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_style_and_visited<style::gecko::wrapper::GeckoElement> servo/components/style/style_resolver.rs:342
7 xul.dll style::style_resolver::StyleResolverForElement<style::gecko::wrapper::GeckoElement>::cascade_primary_style<style::gecko::wrapper::GeckoElement> servo/components/style/style_resolver.rs:239
8 xul.dll geckoservo::glue::Servo_ResolveStyleLazily servo/ports/geckolib/glue.rs:5400
9 xul.dll mozilla::ServoStyleSet::ResolveStyleLazily layout/style/ServoStyleSet.cpp:1086

This appears to be a stylo OOM. I'm not sure if it's actionable but given the non-trivial volume I'd rather have it on file.

Severity: -- → S2

Is it possible to know the size of the allocation causing the crash from Rust code? The value of "oom allocation size" in the reports is empty.

Flags: needinfo?(mh+mozilla)
Priority: -- → P2
Whiteboard: [layout:triage-discuss]

It should be in there. If it isn't, it means https://hg.mozilla.org/mozilla-central/file/161920b70ae464ae0157139327c2d2d7b0480275/memory/mozalloc/mozalloc_oom.cpp#l54 was never called.
In any case, you should be able to find the error string containing the size on the stack in the minidump.

Flags: needinfo?(mh+mozilla)

Emilio: Anything actionable here? (Just revisiting this since it's still open with some regular volume. I don't think we had anyone look at any minidumps.)

Flags: needinfo?(emilio)

The OOM allocation size on the crash report doesn't seem there but I can try to look at some minidumps.

URLs are mostly Youtube / Facebook, which is not super-surprising since they use a lot of custom properties. If looking at minidumps doesn't highlight crazy sizes another thing we may want to do is try to optimize custom property memory usage. I filed bug 1675639 for that.

The dump for the crash in comment 0 has:

"out of memory: 0x0000000000004000 bytes requested"

16k sure are quite a bit, but not totally crazy given the amount of custom properties these can use. Hopefully the patch in bug 1675639 should significantly reduce the excess capacity we allocate. Also it should help regardless, since IndexMap was updated to be a more compact data structure rather than a BTreeMap. A few of the minidumps I looked at had massive btreemaps which after bug 1675639 should be much more compact (just a Vec<> plus table from hash to index).

Flags: needinfo?(emilio)

We should watch the crash volume here given the above, but I hope it to go down.

Whiteboard: [layout:triage-discuss]
Flags: needinfo?(emilio)
Flags: needinfo?(emilio)
See Also: → 1673215
Whiteboard: qa-not-actionable

Copying crash signatures from duplicate bugs.

Crash Signature: [@ mozalloc_abort | style::custom_properties::CustomPropertiesBuilder::cascade] → [@ mozalloc_abort | style::custom_properties::CustomPropertiesBuilder::cascade] [@ style::custom_properties::CustomPropertiesBuilder::cascade]

Crash volume went significantly down and this doesn't seem particularly actionable other than that. The remaining OOMs are here while cloning the map... We could try to use a different representation of the properties? But hard to know how would that look like.

Severity: S2 → S3
Crash Signature: [@ mozalloc_abort | style::custom_properties::CustomPropertiesBuilder::cascade] [@ style::custom_properties::CustomPropertiesBuilder::cascade] → [@ mozalloc_abort | style::custom_properties::CustomPropertiesBuilder::cascade] [@ style::custom_properties::CustomPropertiesBuilder::cascade]
Priority: P2 → P3

There's been signature changes here due to inlined functions, one is std::process::abort | mozglue_static::moz_memory::memalign and the other is std::process::abort | mozglue_static::moz_memory::memalign. Both aren't very specific and need changes to signature generation to become useful.

Crash Signature: [@ mozalloc_abort | style::custom_properties::CustomPropertiesBuilder::cascade] [@ style::custom_properties::CustomPropertiesBuilder::cascade] → [@ mozalloc_abort | style::custom_properties::CustomPropertiesBuilder::cascade] [@ style::custom_properties::CustomPropertiesBuilder::cascade] [@ alloc::raw_vec::RawVec<T>::allocate_in]
You need to log in before you can comment on or make changes to this bug.