Closed Bug 938794 Opened 6 years ago Closed 6 years ago

Annotate OOM sizes for infallible XPCOM structures

Categories

(Core :: XPCOM, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla28
Tracking Status
firefox26 --- unaffected
firefox27 + fixed
firefox28 --- fixed

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files)

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 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.
Attachment #832480 - Flags: review?(nfroyd) → review+
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.
(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.
https://hg.mozilla.org/integration/mozilla-inbound/rev/3bee396bb681

I'd like to take this in 27 as well for OOM analysis.
Target Milestone: --- → mozilla28
https://hg.mozilla.org/mozilla-central/rev/3bee396bb681
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
[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
Attachment #8341747 - Flags: approval-mozilla-aurora?
Attachment #8341747 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: checkin-needed
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.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.