Open Bug 811483 Opened 12 years ago Updated 2 years ago

Annotate crash reports with the size of last failed malloc

Categories

(Core :: General, defect)

defect

Tracking

()

People

(Reporter: bjacob, Unassigned)

References

Details

Fallible malloc is nice, but it does make OOM crashes tricky to understand. What we could do is whenever malloc returns null, update a timestamp, and add that timestamp as a new field in crash reports. We would then informally identify OOM crashes as the crashes where the last-failed-malloc timestamp is less than, say, 10 seconds before the crash. The malloc instrumentation should use the work that being done in bug 804303.
Depends on: 811519
Note: if annotating a crash reports relies on the ability to dynamically allocate memory, we will probably want to do that only for malloc sizes above a certain threshold.
As discussed in bug 804303, doing this by default depends on enabling jemalloc 3 by default, unless we decide to do bug 804303 for mozjemalloc (very unlikely).
Note that we already provide an OOMAllocationSize annotation for when mozalloc_handle_oom is called: http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#1005 http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/nsExceptionHandler.cpp#409 http://mxr.mozilla.org/mozilla-central/source/memory/mozalloc/mozalloc_oom.cpp#18 It contains the size of the allocation that caused the OOM. I'm not sure that this information is currently visible in the public crash reports, but that could be fixed.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3) > Note that we already provide an OOMAllocationSize annotation for when > mozalloc_handle_oom is called: > http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/ > nsExceptionHandler.cpp#1005 > http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/ > nsExceptionHandler.cpp#409 > http://mxr.mozilla.org/mozilla-central/source/memory/mozalloc/mozalloc_oom. > cpp#18 > > It contains the size of the allocation that caused the OOM. I'm not sure > that this information is currently visible in the public crash reports, but > that could be fixed. Is there anything sensitive about doing that?
(In reply to Benoit Jacob [:bjacob] from comment #2) > As discussed in bug 804303, doing this by default depends on enabling > jemalloc 3 by default, unless we decide to do bug 804303 for mozjemalloc > (very unlikely). It looks like it was fixed? Can this bug proceed?
cc+ our crash master (Kairo)
OOMAllocationSize is already in the public reports by default. But note that this is only annotated if we are using the *infallible* form of malloc. If we are using the fallible form, we don't annotate. I do think we should fix this to annotate on *every* failed malloc, but it will require jemalloc and it may not be simple to do. And it doesn't require any allocation (because we'd be doing that inside the allocator lock, which is bad!). It just sets a global variable which is then inserted into the report.
(In reply to Benjamin Smedberg [:bsmedberg] from comment #7) > OOMAllocationSize is already in the public reports by default. But note that > this is only annotated if we are using the *infallible* form of malloc. If > we are using the fallible form, we don't annotate. I do think we should fix > this to annotate on *every* failed malloc, Exactly and this is the point of the present bug. > but it will require jemalloc and > it may not be simple to do. That was until bug 804303 landed. The new replace-malloc infrastructure in bug 804303 is the right setting to add such allocator instrumentation regardless of which allocator is actually used, and regardless of the platform. See the conversation in bug 820133.
Summary: Annotate crash reports with the timestamp of last failed malloc → Annotate crash reports with the size of last failed malloc
Hey, I figured something out ;-) The key presses are being thrown out because of a check at this location: http://hg.mozilla.org/mozilla-central/annotate/67f2a2816651/content/events/src/nsEventStateManager.cpp#l3510 if (nsEventStatus_eConsumeNoDefault != *aStatus)
And the event has mDefaultPrevented being set on it because of this stack: > xul.dll!nsDOMEvent::PreventDefault() Line 431 C++ xul.dll!nsDOMKeyboardEvent::PreventDefault() Line 25 C++ xul.dll!nsPluginInstanceOwner::DispatchKeyToPlugin(aKeyEvent=0x0569a3f0) Line 1834 C++ xul.dll!nsPluginInstanceOwner::HandleEvent(aEvent=0x0569a3f0) Line 1942 C++
Please ignore comment 9-10, that was for another bug.
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.