Closed Bug 938794 Opened 7 years ago Closed 7 years ago
Annotate OOM sizes for infallible XPCOM structures
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.
Status: NEW → RESOLVED
Closed: 7 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+
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.
You need to log in before you can comment on or make changes to this bug.