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

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Calixte Denizet (:calixte)]

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

Comment 1

[Henri Sivonen (:hsivonen)]

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.

Comment 2

[Henri Sivonen (:hsivonen)]

This would benefit from bug 1482828.

Comment 3

[Calixte Denizet (:calixte)]

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.

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

There is a frequent reproducibility here by trying to request new jobs on Treeherder, e.g. for 8920abb9801d at https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=success&filter-resultStatus=pending&filter-resultStatus=running&group_state=expanded&tochange=34f564520a234b4318cce5be5c86adcd2be8187b&filter-searchStr=windows%20ccov%20marionette&fromchange=0dcc490db922fc504a1b92339be000260448d75b

Comment 7

[Henri Sivonen (:hsivonen)]

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.

Comment 8

[Henri Sivonen (:hsivonen)]

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6bac87f5027cdcb2d96c13d955aa79bcb4eced2e

Comment 9

[Henri Sivonen (:hsivonen)]

Failed:
https://treeherder.mozilla.org/logviewer.html#?job_id=194313020&repo=try&lineNumber=25465

Comment 10

[Henri Sivonen (:hsivonen)]

Another try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e1425c9a97849653b556ee10410648b4dc0fe5b5

Comment 11

[Henri Sivonen (:hsivonen)]

Hopefully less bogus try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e27259dc0da88a12396e93481628eff84241f1fc

Comment 12

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1483603 - Use BulkWrite instead of write past length via BeginWriting() in XHR.

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

Comment 13

[Henri Sivonen (:hsivonen)]

I'm going to pursue this as a string bug instead.

Comment 14

[Henri Sivonen (:hsivonen)]

Try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=136eea70b238f5f47d95bf413d1296fe1e1e7bd1

Comment 15

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1483603 - Avoid propagating OOM from StartBulkWrite() when shrinking the buffer.

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

Comment 16

[Nathan Froyd [:froydnj]]

Comment on attachment 9002709 [details]
Bug 1483603 - Avoid propagating OOM from StartBulkWrite() when shrinking the buffer.

Nathan Froyd [:froydnj] has approved the revision.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c6a80a1c10a1
Avoid propagating OOM from StartBulkWrite() when shrinking the buffer. r=froydnj

Comment 18

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/c6a80a1c10a1