Closed
Bug 1110499
Opened 9 years ago
Closed 9 years ago
Crash reporter does not work in Android L
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox34+ verified, firefox35+ verified, firefox36+ verified, firefox37+ verified, firefox-esr31 wontfix, fennec34+)
People
(Reporter: snorp, Assigned: snorp)
References
Details
Attachments
(2 files, 2 obsolete files)
1.80 KB,
patch
|
snorp
:
review+
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
lmandel
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
1.19 KB,
patch
|
snorp
:
review+
lsblakk
:
approval-mozilla-beta+
lsblakk
:
approval-mozilla-release+
|
Details | Diff | Splinter Review |
We've had a few reports of users having Firefox just dump them to the home screen on Android L. On my own phone, I have no reported crashes, and sending a SEGV to it via adb does not invoke the crash reporter. It does on my Kit Kat phone.
Assignee | ||
Updated•9 years ago
|
tracking-fennec: --- → ?
Assignee | ||
Updated•9 years ago
|
status-firefox34:
--- → affected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
status-firefox37:
--- → affected
status-firefox-esr31:
--- → affected
Assignee | ||
Comment 1•9 years ago
|
||
I have confirmed on a local build with the crash reporter forced on (MOZ_CRASHREPORTER=1 in environment) and MOZ_CRASH() that the reporter is not working on L. The same build works fine on Kit Kat.
Assignee | ||
Comment 2•9 years ago
|
||
OK, the good news is that minidumps appear to be written correctly. They are all in the minidumps directory in my profile. We just aren't launching the crash reporter for some reason. Continuing to investigate.
Assignee | ||
Comment 3•9 years ago
|
||
It looks like Android kills our crash reporter process as part of crash recovery. The execve() to run 'am start' is happening, but right after I see a log message indicating that Android is killing that process. See below: I/SNORP (29533): execing am with user 0 I/libprocessgroup( 400): Killing pid 29533 in uid 10102 as part of process group 29406 I'm trying to change the process group to avoid this, but it doesn't seem to be helping so far.
Assignee | ||
Comment 4•9 years ago
|
||
This patch makes us wait on the 'am start' command to complete before exiting the signal handler.
Assignee: nobody → snorp
Attachment #8535837 -
Flags: review?(ted)
Attachment #8535837 -
Flags: review?(blassey.bugs)
Assignee | ||
Comment 5•9 years ago
|
||
[Tracking Requested - why for this release]: We need this to enable the crash reporter on Android Lollipop
tracking-firefox34:
--- → ?
tracking-firefox35:
--- → ?
tracking-firefox36:
--- → ?
tracking-firefox37:
--- → ?
Updated•9 years ago
|
Attachment #8535837 -
Flags: review?(blassey.bugs) → review+
Comment 6•9 years ago
|
||
FWIW, I wanted to test that our crash reporting process is working when my Nexus 10 was updated to L, but I found that crashme isn't working and I couldn't find anything else that would reliably crash in a way that is supposed to get the crash reporter (and not just makes us get killed by the LMK). Thanks for finding this!
Updated•9 years ago
|
Comment 7•9 years ago
|
||
Comment on attachment 8535837 [details] [diff] [review] Don't exit crash handler on Android until we've finished started the reporter Review of attachment 8535837 [details] [diff] [review]: ----------------------------------------------------------------- Glad this wound up being a simple fix! ::: toolkit/crashreporter/nsExceptionHandler.cpp @@ +963,5 @@ > + // be killed by the ActivityManager as soon as the signal handler exits > + int r, status; > + do { > + r = sys_waitpid(pid, &status, __WALL); > + } while (r == -1 && errno == EINTR); You can use EINTR_WRAPPER for this: http://dxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/google-breakpad/src/common/linux/eintr_wrapper.h
Attachment #8535837 -
Flags: review?(ted) → review+
Comment 8•9 years ago
|
||
Er, the macro is HANDLE_EINTR.
Assignee | ||
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/14a3a1d0dd40
Updated•9 years ago
|
Status: NEW → ASSIGNED
Flags: in-testsuite?
Flags: in-moztrap?(fennec)
Comment 10•9 years ago
|
||
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #6) > FWIW, I wanted to test that our crash reporting process is working when my > Nexus 10 was updated to L, but I found that crashme isn't working and I > couldn't find anything else that would reliably crash in a way that is > supposed to get the crash reporter (and not just makes us get killed by the > LMK). Thanks for finding this! FYI I fixed crashme to work on Android again: http://people.mozilla.com/~tmielczarek/crashme/
Comment 11•9 years ago
|
||
Tracking for Fx34 in case we make a respin to get bug 1108627 too
tracking-fennec: ? → 34+
Comment 12•9 years ago
|
||
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #10) > FYI I fixed crashme to work on Android again: > http://people.mozilla.com/~tmielczarek/crashme/ Cool, I can now confirm that it crashes today's Nightly on Android L (Nexus 10) without a crash reporter coming up. Once the patch has landed in a Nightly build, I'll re-test. :)
Comment 13•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/14a3a1d0dd40
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
Assignee | ||
Comment 14•9 years ago
|
||
This is the patch that I ended up checking in
Attachment #8535837 -
Attachment is obsolete: true
Attachment #8537824 -
Flags: review+
Assignee | ||
Comment 15•9 years ago
|
||
Comment on attachment 8537824 [details] [diff] [review] Don't exit crash handler on Android until we've finished started the reporter r=ted Approval Request Comment [Feature/regressing bug #]: N/A (Android Lollipop release) [User impact if declined]: No crash reports on Lollipop [Describe test coverage new/current, TBPL]: local testing on Nightly [Risks and why]: Low [String/UUID change made/needed]: None
Attachment #8537824 -
Flags: approval-mozilla-release?
Attachment #8537824 -
Flags: approval-mozilla-beta?
Attachment #8537824 -
Flags: approval-mozilla-aurora?
Comment 16•9 years ago
|
||
Comment on attachment 8537824 [details] [diff] [review] Don't exit crash handler on Android until we've finished started the reporter r=ted This is the driver for Firefox for Android 34.0.1. Although this change hasn't been on m-c very long, it is a high enough win that we should take this everywhere - including release.
Attachment #8537824 -
Flags: approval-mozilla-release?
Attachment #8537824 -
Flags: approval-mozilla-release+
Attachment #8537824 -
Flags: approval-mozilla-beta?
Attachment #8537824 -
Flags: approval-mozilla-beta+
Attachment #8537824 -
Flags: approval-mozilla-aurora?
Attachment #8537824 -
Flags: approval-mozilla-aurora+
Updated•9 years ago
|
Comment 17•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/7660acc7c6d3 https://hg.mozilla.org/releases/mozilla-beta/rev/d2969753f2c9 https://hg.mozilla.org/releases/mozilla-release/rev/8e81c96d0749
Comment 18•9 years ago
|
||
It seems that the issue is still reproducing on Firefox 34.0.1 Note: Verified as fixed on latest Nightly on Nexus 5(Android 5.0) and Nexus 7(Android 5.0.1)
Assignee | ||
Comment 20•9 years ago
|
||
I found the problem. The crash reporter activity is getting launched into the crashed process instead of a new one. It works on Nightly because we have the patch from bug 1043457 there, which assigns the crash reporter activity to its own process. I don't think we want that whole patch for release, just that tidbit. Testing now.
Flags: needinfo?(snorp)
Assignee | ||
Comment 21•9 years ago
|
||
Approval Request Comment [Feature/regressing bug #]: N/A [User impact if declined]: no crash reporter on L [Describe test coverage new/current, TBPL]: local [Risks and why]: very low [String/UUID change made/needed]: None We need this for both 34 and 35. 36+ is unaffected.
Attachment #8538522 -
Flags: review?(nchen)
Attachment #8538522 -
Flags: approval-mozilla-release?
Attachment #8538522 -
Flags: approval-mozilla-beta?
Comment 22•9 years ago
|
||
Comment on attachment 8538522 [details] [diff] [review] Put the CrashReporter activity in its own process Review of attachment 8538522 [details] [diff] [review]: ----------------------------------------------------------------- ::: mobile/android/base/AndroidManifest.xml.in @@ +345,5 @@ > #endif > > #if MOZ_CRASHREPORTER > <activity android:name="org.mozilla.gecko.CrashReporter" > + android:process="@ANDROID_PACKAGE_NAME@.CrashReporter" "@MANGLED_ANDROID_PACKAGE_NAME@.CrashReporter"?
Attachment #8538522 -
Flags: review?(nchen) → review+
Assignee | ||
Comment 23•9 years ago
|
||
carrying r+
Attachment #8538522 -
Attachment is obsolete: true
Attachment #8538522 -
Flags: approval-mozilla-release?
Attachment #8538522 -
Flags: approval-mozilla-beta?
Attachment #8538575 -
Flags: review+
Assignee | ||
Comment 24•9 years ago
|
||
Comment on attachment 8538575 [details] [diff] [review] Put the CrashReporter activity in its own process r=jchen Approval Request Comment [Feature/regressing bug #]: N/A [User impact if declined]: no crash reporter on L [Describe test coverage new/current, TBPL]: local [Risks and why]: very low [String/UUID change made/needed]: None We need this for both 34 and 35. 36+ is unaffected.
Attachment #8538575 -
Flags: approval-mozilla-release?
Attachment #8538575 -
Flags: approval-mozilla-beta?
Updated•9 years ago
|
Attachment #8538575 -
Flags: approval-mozilla-release?
Attachment #8538575 -
Flags: approval-mozilla-release+
Attachment #8538575 -
Flags: approval-mozilla-beta?
Attachment #8538575 -
Flags: approval-mozilla-beta+
Comment 25•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/3e27ff97b852 https://hg.mozilla.org/releases/mozilla-release/rev/a15a4744b6bf
Comment 26•9 years ago
|
||
Verified as fixed on Firefox 34.0.1 RC build2 and on latest Aurora on Nexus 5 (Android 5.0)
Comment 27•9 years ago
|
||
Verified as fixed on Firefox 35 Beta 6 on Nexus 7 (Android 5.0.1)
Status: RESOLVED → VERIFIED
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•