938794 - Annotate OOM sizes for infallible XPCOM structures

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Benjamin Smedberg]

We have infallible XPCOM data structures which currently do NS_RUNTIMEABORT("OOM") if allocation fails. This means that the allocation size isn't annotated in the crash report, and so it's hard to distinguish cases where we're allocating stupidly-large data from other cases. See bug 767343 for some background.

Comment 1

[Benjamin Smedberg]

Attachment: Annotate OOM size as infallible string or data structures abort

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 832480 [details] [diff] [review]
Annotate OOM size as infallible string or data structures abort

Review of attachment 832480 [details] [diff] [review]:
-----------------------------------------------------------------

This is a little weird, since in most of these cases, you're not really reporting the actual allocation, but some number from which the actual allocation can be derived.  I guess the reasoning here is that the actual allocation size and the already-allocated size are close enough (a factor of 2 or so) that they can stand in for each other?  That looks like a reasonable assumption.

Comment 3

[Benjamin Smedberg]

Comment on attachment 832480 [details] [diff] [review]
Annotate OOM size as infallible string or data structures abort

In almost all these cases the size is known: most of the methods that report mLength are in-place mutations where we're just converting the potentially sharable string buffer to a mutable string buffer of the same legnth, so the allocation of mLength is accurate (minus one, I guess).

The only case where the accurate length isn't really available is the UTF8<->UTF16 conversion routines where the actual needed size is buried in conversion routine, but the estimation there ought to be good enough.

Comment 4

[Nathan Froyd [:froydnj]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #3)
> Comment on attachment 832480 [details] [diff] [review]
> Annotate OOM size as infallible string or data structures abort
> 
> In almost all these cases the size is known

OK, great!  Thanks for the clarification.

Comment 5

[Benjamin Smedberg]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3bee396bb681

I'd like to take this in 27 as well for OOM analysis.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/3bee396bb681

Comment 7

[Benjamin Smedberg]

Attachment: Fx27 branch patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Better data needed on OOM crashes
User impact if declined: not knowing which crashes are important
Testing completed (on m-c, etc.): landed, no known regressions, 
Risk to taking this patch (and alternatives if risky): low risk - code runs only in OOM cases where we're going to crash anyway
String or IDL/UUID changes made by this patch: None

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ec081a4840ca

Comment 9

[u279076]

I don't think this needs QA verification. If anyone thinks that's a mistake please remove the [qa-] whiteboard tag and add the verifyme keyword.