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)

All
Android
defect
Not set
normal

Tracking

(firefox34+ verified, firefox35+ verified, firefox36+ verified, firefox37+ verified, firefox-esr31 wontfix, fennec34+)

VERIFIED FIXED
Firefox 37
Tracking Status
firefox34 + verified
firefox35 + verified
firefox36 + verified
firefox37 + verified
firefox-esr31 --- wontfix
fennec 34+ ---

People

(Reporter: snorp, Assigned: snorp)

References

Details

Attachments

(2 files, 2 obsolete files)

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.
tracking-fennec: --- → ?
Blocks: android-l
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.
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.
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.
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)
[Tracking Requested - why for this release]: We need this to enable the crash reporter on Android Lollipop
Attachment #8535837 - Flags: review?(blassey.bugs) → review+
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!
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+
Er, the macro is HANDLE_EINTR.
Status: NEW → ASSIGNED
Flags: in-testsuite?
Flags: in-moztrap?(fennec)
(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/
Tracking for Fx34 in case we make a respin to get bug 1108627 too
tracking-fennec: ? → 34+
(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. :)
https://hg.mozilla.org/mozilla-central/rev/14a3a1d0dd40
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 37
This is the patch that I ended up checking in
Attachment #8535837 - Attachment is obsolete: true
Attachment #8537824 - Flags: review+
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 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+
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)
Snorp, can you please take a look on this?
Flags: needinfo?(snorp)
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)
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 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+
carrying r+
Attachment #8538522 - Attachment is obsolete: true
Attachment #8538522 - Flags: approval-mozilla-release?
Attachment #8538522 - Flags: approval-mozilla-beta?
Attachment #8538575 - Flags: review+
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?
Attachment #8538575 - Flags: approval-mozilla-release?
Attachment #8538575 - Flags: approval-mozilla-release+
Attachment #8538575 - Flags: approval-mozilla-beta?
Attachment #8538575 - Flags: approval-mozilla-beta+
Verified as fixed on Firefox 34.0.1 RC build2 and on latest Aurora on Nexus 5 (Android 5.0)
Verified as fixed on Firefox 35 Beta 6 on Nexus 7 (Android 5.0.1)
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.