1484045 - Crash in OOM | unknown | NS_ABORT_OOM | bool `anonymous namespace'::StringBuilder::ToString called by nsContentUtils::SerializeNodeToMarkup()

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[Andrew McCreight [:mccr8]]

This bug was filed from the Socorro interface and is
report bp-63f44ad2-c330-4622-9ecb-b80570180815.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll NS_ABORT_OOM xpcom/base/nsDebugImpl.cpp:624
1 xul.dll bool `anonymous namespace'::StringBuilder::ToString dom/base/nsContentUtils.cpp:9180
2 xul.dll nsContentUtils::SerializeNodeToMarkup dom/base/nsContentUtils.cpp
3 xul.dll mozilla::dom::FragmentOrElement::GetMarkup dom/base/FragmentOrElement.cpp:2136
4 xul.dll static bool mozilla::dom::Element_Binding::get_outerHTML dom/bindings/ElementBinding.cpp:3531
5 xul.dll mozilla::dom::binding_detail::GenericGetter<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions> dom/bindings/BindingUtils.cpp:3187
6 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:533
7 xul.dll static bool InternalCall js/src/vm/Interpreter.cpp:584
8 xul.dll js::Call js/src/vm/Interpreter.cpp:603
9 xul.dll JS::Call js/src/jsapi.cpp:2915

=============================================================

This showed up in the 20180815100249 build. 19 crashes from 3 installations.

There's also a variant that shows up with "large" instead of unknown, such as this crash: bp-ad4d79ba-b88d-46db-b942-c0ac90180816

That first showed up in the 20180814220344 build with 11 crashes from 5 installations.

Comment 1

[Andrew McCreight [:mccr8]]

Henri, could this be related to bug 1483603 or some other thing you've been working on?

Comment 2

[Henri Sivonen (:hsivonen)]

This is fallout from bug 1402247. The code in nsContentUtils.h assumes that none of the appends causes an allocation after the fallible SetCapacity(), but bug 1402247 changed that. I'm pretty sure the bug is the boolean right about here:
https://searchfox.org/mozilla-central/source/servo/support/gecko/nsstring/src/conversions.rs#145

I.e. the code that tries to re-use the pre-existing buffer when *assigning* Latin1 to UTF-16 ends up reallocating when *appending* Latin1 to UTF-16 with prior SetCapacity().

Comment 3

[Henri Sivonen (:hsivonen)]

Aside: There's an additional bug here: entity-encoding expands the length but those use infallible appends. StringBuilder::ToString would be a great candidate for the new API from bug 1482828.

Comment 4

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #3)
> Aside: There's an additional bug here: entity-encoding expands the length
> but those use infallible appends. StringBuilder::ToString would be a great
> candidate for the new API from bug 1482828.

Filed bug 1484143 for the pre-existing bug.

Comment 5

[Henri Sivonen (:hsivonen)]

(In reply to Henri Sivonen (:hsivonen) from comment #3)
> StringBuilder::ToString would be a great
> candidate for the new API from bug 1482828.

This doesn't actually make sense.

Comment 6

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1484045 - Prohibit shrinking of the string's buffer in appends of Latin1 to UTF-16 and vice versa.

This restores the old allocation semantics for "append" operations between
Latin1 and UTF-16 while keeping the buffer re-use optimization for the
"assign" cases.

MozReview-Commit-ID: 8JCw3AaCNLN

Comment 7

[Nathan Froyd [:froydnj]]

Comment on attachment 9001858 [details]
Bug 1484045 - Prohibit shrinking of the string's buffer in appends of Latin1 to UTF-16 and vice versa.

Nathan Froyd [:froydnj] has approved the revision.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9494c03553b9
Prohibit shrinking of the string's buffer in appends of Latin1 to UTF-16 and vice versa. r=froydnj

Comment 9

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/9494c03553b9