1042984 - Add extensive logging and descriptive crash data for library load errors

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Richard Newman [:rnewman]]

In aid of fixing Bug 1042935.

Comment 1

[Richard Newman [:rnewman]]

Howzat?

LOAD mozglue Data: /data/data/org.mozilla.fennec_rnewman, ax=false, ddx=true, -1x=false, -2x=false, nativeLib: /data/app-lib/org.mozilla.fennec_rnewman-1, dirx=true, libx=true

Reports:

context.getApplicationInfo().dataDir
Whether /data/app-lib/... exists
Whether /data/data/... exists
Whether /data/data/org.mozilla.firefox-{1,2} exist
The system-reported nativeLib path
Whether that exists
Whether libmozglue.so is in that path.

I'm also adding code to load from the nativeLib path, which is what System.loadLibrary() *should* do, but hey.



07-23 16:22:01.777 F/GeckoLoader(19732): Couldn't load mozglue. Trying /data/data/org.mozilla.fennec_rnewman/lib/libmozglue.so
07-23 16:22:01.797 F/GeckoLoader(19732): Couldn't load mozglue. Trying /data/app-lib path.
07-23 16:22:01.797 F/GeckoLoader(19732): Couldn't load mozglue: java.lang.Exception: foo. Trying /data/data path.
07-23 16:22:01.807 F/GeckoLoader(19732): Failed every attempt to load mozglue. Giving up.
07-23 16:22:01.817 E/GeckoLoader(19732): Load diagnostics: LOAD mozglue Data: /data/data/org.mozilla.fennec_rnewman, ax=false, ddx=true, -1x=false, -2x=false, nativeLib: /data/app-lib/org.mozilla.fennec_rnewman-1, dirx=true, libx=true
07-23 16:22:01.817 E/Settings:HtcWrapHeaderInfo(19538): updateDevelopment, bPrefShow = true
07-23 16:22:01.817 E/AndroidRuntime(19732): FATAL EXCEPTION: main
07-23 16:22:01.817 E/AndroidRuntime(19732): Process: org.mozilla.fennec_rnewman, PID: 19732
07-23 16:22:01.817 E/AndroidRuntime(19732): java.lang.RuntimeException: Unable to create application org.mozilla.gecko.GeckoApplication: java.lang.RuntimeException: LOAD mozglueData: /data/data/org.mozilla.fennec_rnewman, ax=false, ddx=true, -1x=false, -2x=false, nativeLib: /data/app-lib/org.mozilla.fennec_rnewman-1, dirx=true, libx=true

Comment 2

[Richard Newman [:rnewman]]

Attachment: Add extensive logging and descriptive crash data for library load errors. v1

You'll note that this does a little more than just logging. It also tries to fix the bug, and:

* Makes the logic more readable.
* Adds another case that might save our asses.
* Requires passing a context in, which means altering PasswordsProvider.
* Adds some finals.
* Fixes a little whitespace and some incorrect comments.

Comment 3

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/fx-team/rev/b7420e11f73b

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/b7420e11f73b

Comment 5

[Richard Newman [:rnewman]]

Comment on attachment 8461228 [details] [diff] [review]
Add extensive logging and descriptive crash data for library load errors. v1

Approval Request Comment
[Feature/regressing bug #]:
  Probably an Android bug. Prior history in Bug 990130.

[User impact if declined]:
  We're seeing a high early crash rate on Google Play. We don't know exactly why, but it looks like Android's APK-layering story is screwing up.

[Describe test coverage new/current, TBPL]:
  Landed on Nightly, so we can at least verify that we haven't broken the success case. This is a speculative fix, so we'd uplift in stages. Eventually it needs to hit Beta so we can get data. If it does no harm, we'll do Release too, in case we spin a dot release.

[Risks and why]: 
  See above.

  The risk is that we regress library loading even further for affected users. Right now they crash immediately on startup, though, so we can hardly make things worse!

[String/UUID change made/needed]:
  None.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

We don't have too many users in aurora, so accepting for beta to make sure it gets into beta 2.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ed9f1e3a224d
https://hg.mozilla.org/releases/mozilla-beta/rev/3808d8fbe348

Do we need to consider this for esr31 now too given it's new status for Android?

Comment 8

[Richard Newman [:rnewman]]

Let's do so for the fix -- it might be this patch, or it might not!

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Richard, so, do you know if it this patch or not? ;)

Comment 10

[Richard Newman [:rnewman]]

We just uplifted Bug 1046369, which goes a step further.

There's no harm in sending this particular bug to ESR, and it almost certainly helps, but Bug 1042935 tracks the point at which we're no longer worried!

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8461228 [details] [diff] [review]
Add extensive logging and descriptive crash data for library load errors. v1

This fix shipped this week in Firefox 32. As such, I'm denying the approval request for release as it is no longer necessary.

Comment 12

[Lawrence Mandel [:lmandel] (use needinfo)]

Richard - We're going to ship Fennec 31.1.0 this week without this fix. How important is this for Fennec 31.x?

Comment 13

[Richard Newman [:rnewman]]

(In reply to Lawrence Mandel [:lmandel] from comment #12)
> Richard - We're going to ship Fennec 31.1.0 this week without this fix. How
> important is this for Fennec 31.x?

Are we shipping that via Play?

If so, I'd take it: Play has as much chance of screwing up that APK install as every other, and it leaves the app in a broken state.

Comment 14

[Lawrence Mandel [:lmandel] (use needinfo)]

(In reply to Richard Newman [:rnewman] from comment #13)
> (In reply to Lawrence Mandel [:lmandel] from comment #12)
> > Richard - We're going to ship Fennec 31.1.0 this week without this fix. How
> > important is this for Fennec 31.x?
> 
> Are we shipping that via Play?

Yes.

> If so, I'd take it: Play has as much chance of screwing up that APK install
> as every other, and it leaves the app in a broken state.

This may be possible as I don't yet have the APK. Can you please submit an uplift request for ESR31 and I'll follow up once we're ready to build.

Comment 15

[Richard Newman [:rnewman]]

Comment on attachment 8461228 [details] [diff] [review]
Add extensive logging and descriptive crash data for library load errors. v1

[Approval Request Comment]

See comment 14.


(In reply to Lawrence Mandel [:lmandel] from comment #14)

> > Are we shipping that via Play?
> 
> Yes.

Can you point me to where? I don't see any APK lower than 32 in the admin console, nor do I see a separate ESR app from a user's perspective.

I'd like to take a look at the crash stats for ESR.

Comment 16

[Kevin Brosnan [Ex-Mozilla]]

Do we really need this in our ESR builds? They are going to be shortlived and will be delivered to a rather small population on ARMv6 phones and/or running Android 2.2. We don't have any crash stats for ESR as the build has yet to be delivered to the users. It will be a thing early next week if all goes smoothly.

Comment 17

[Benjamin Kerensa [:bkerensa]]

Unless you can show that the users on the builds coming off the ESR branch are impacted by this we can't take major churn and this really doesn't need to be in ESR.