Closed Bug 1483603 Opened 2 years ago Closed 1 year ago

Crash in OOM | large | NS_ABORT_OOM | nsTSubstring<T>::SetCapacity

Categories

(Core :: String, defect, critical)

x86
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- fixed

People

(Reporter: calixte, Assigned: hsivonen)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file, 1 obsolete file)

This bug was filed from the Socorro interface and is
report bp-4c5e8fc9-b834-47fa-a230-6af1e0180815.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll NS_ABORT_OOM xpcom/base/nsDebugImpl.cpp:624
1 xul.dll nsTSubstring<char16_t>::SetCapacity xpcom/string/nsTSubstring.cpp:746
2 xul.dll nsresult mozilla::dom::XMLHttpRequestMainThread::AppendToResponseText dom/xhr/XMLHttpRequestMainThread.cpp:536
3 xul.dll static nsresult mozilla::dom::XMLHttpRequestMainThread::StreamReaderFunc dom/xhr/XMLHttpRequestMainThread.cpp:1602
4 xul.dll nsStringInputStream::ReadSegments xpcom/io/nsStringStream.cpp:270
5 xul.dll mozilla::dom::XMLHttpRequestMainThread::OnDataAvailable dom/xhr/XMLHttpRequestMainThread.cpp:1787
6 xul.dll mozilla::net::nsStreamListenerWrapper::OnDataAvailable netwerk/base/nsStreamListenerWrapper.h:31
7 xul.dll nsCORSListenerProxy::OnDataAvailable netwerk/protocol/http/nsCORSListenerProxy.cpp:678
8 xul.dll mozilla::net::InterceptFailedOnStop::OnDataAvailable netwerk/protocol/http/HttpBaseChannel.cpp:1231
9 xul.dll nsresult mozilla::net::nsHTTPCompressConv::do_OnDataAvailable netwerk/streamconv/converters/nsHTTPCompressConv.cpp:528

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

There are 10 crashes (from 6 installations) in nightly 63 starting with buildid 20180814220344. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1402247.

[1] https://hg.mozilla.org/mozilla-central/rev?node=4ef0f163fdeb
Flags: needinfo?(hsivonen)
Crash Signature: [@ OOM | large | NS_ABORT_OOM | nsTSubstring<T>::SetCapacity] → [@ OOM | large | NS_ABORT_OOM | nsTSubstring<T>::SetCapacity] [@ OOM | large | NS_ABORT_OOM | nsresult mozilla::dom::XMLHttpRequestMainThread::AppendToResponseText]
Is there any reason to believe that this is a new bug and not a legitimate OOM situation? I guess we should make XHR use a fallible allocation for this in any case.
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Flags: needinfo?(hsivonen) → needinfo?(cdenizet)
This would benefit from bug 1482828.
Depends on: 1482828
For signature "OOM | large | NS_ABORT_OOM | nsTSubstring<T>::SetCapacity", the last (before the 2 we've here) buildid with 1 crash is 20180312220041 (on nightly).
And for the other signature, there was no crash before.
I don't know if it's a legitimate OOM situation but at least for the first signature we don't have this crash on nightly since a long time, so it could be a coincidence or a clue that something is wrong.
Flags: needinfo?(cdenizet)
Duplicate of this bug: 1483530
Duplicate of this bug: 1483742
So this is an especially bad case of bug 1472113 that I somehow missed in my audit of our code.

I'm going to develop a fix based on bug 1482828 on the assumption that that one can land soon. froydnj, please let me know if that's not a reasonable assumption.
Flags: needinfo?(nfroyd)
Component: String → DOM
Depends on: 1483764
The old code assumes that it's OK to use nsAString::BeginWriting() to write
past the string's logical length if the string has enough capacity. This is
bogus, because the string doesn't know of data written past its logical
length.

The BulkWrite API has been created precisely for this purpose and allows
orderly capacity-aware low-level writes to the string.

MozReview-Commit-ID: FkKz5rBQu19
I'm going to pursue this as a string bug instead.
Component: DOM → String
Flags: needinfo?(nfroyd)
No longer depends on: 1482828
Attachment #9001908 - Attachment is obsolete: true
Shrinking the buffer is purely a memory footprint optimization and can be
omitted as far as the string semantics visible to the caller are concerned.
Since shrinking is optional, it doesn't make sense to propagate error when
it fails due to OOM.

MozReview-Commit-ID: BuyBLCBmYzZ
Crash Signature: [@ OOM | large | NS_ABORT_OOM | nsTSubstring<T>::SetCapacity] [@ OOM | large | NS_ABORT_OOM | nsresult mozilla::dom::XMLHttpRequestMainThread::AppendToResponseText] → [@ OOM | large | NS_ABORT_OOM | nsTSubstring<T>::SetCapacity] [@ OOM | large | NS_ABORT_OOM | nsresult mozilla::dom::XMLHttpRequestMainThread::AppendToResponseText] [@ OOM | large | NS_ABORT_OOM | mozilla::dom::XMLHttpRequestMainThread::AppendToResponse…
Comment on attachment 9002709 [details]
Bug 1483603 - Avoid propagating OOM from StartBulkWrite() when shrinking the buffer.

Nathan Froyd [:froydnj] has approved the revision.
Attachment #9002709 - Flags: review+
Pushed by hsivonen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c6a80a1c10a1
Avoid propagating OOM from StartBulkWrite() when shrinking the buffer. r=froydnj
https://hg.mozilla.org/mozilla-central/rev/c6a80a1c10a1
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Depends on: 1486470
Crash Signature: mozilla::dom::XMLHttpRequestMainThread::AppendToResponseText] → mozilla::dom::XMLHttpRequestMainThread::AppendToResponseText] [@ OOM | unknown | NS_ABORT_OOM | mozilla::dom::XMLHttpRequestMainThread::AppendToResponseText]
Depends on: 1519686
You need to log in before you can comment on or make changes to this bug.