Add extensive logging and descriptive crash data for library load errors

RESOLVED FIXED in Firefox 32

Status

()

defect
--
critical
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: rnewman, Assigned: rnewman)

Tracking

(Blocks 2 bugs)

31 Branch
Firefox 34
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox31 wontfix, firefox32+ fixed, firefox33+ fixed, firefox34+ fixed, fennec32+)

Details

Attachments

(1 attachment)

In aid of fixing Bug 1042935.
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
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.
Attachment #8461228 - Flags: review?(mark.finkle)
Attachment #8461228 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/mozilla-central/rev/b7420e11f73b
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
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.
Attachment #8461228 - Flags: approval-mozilla-release?
Attachment #8461228 - Flags: approval-mozilla-beta?
Attachment #8461228 - Flags: approval-mozilla-aurora?
Attachment #8461228 - Flags: approval-mozilla-beta?
Attachment #8461228 - Flags: approval-mozilla-beta+
Attachment #8461228 - Flags: approval-mozilla-aurora?
Attachment #8461228 - Flags: approval-mozilla-aurora+
We don't have too many users in aurora, so accepting for beta to make sure it gets into beta 2.
Let's do so for the fix -- it might be this patch, or it might not!
Blocks: 1046369
Blocks: 1046373
Richard, so, do you know if it this patch or not? ;)
Flags: needinfo?(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!
Flags: needinfo?(rnewman)
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.
Attachment #8461228 - Flags: approval-mozilla-release? → approval-mozilla-release-
Richard - We're going to ship Fennec 31.1.0 this week without this fix. How important is this for Fennec 31.x?
Flags: needinfo?(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.
Flags: needinfo?(rnewman)
(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 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.
Attachment #8461228 - Flags: approval-mozilla-esr31?
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.
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.
Attachment #8461228 - Flags: approval-mozilla-esr31? → approval-mozilla-esr31-
You need to log in before you can comment on or make changes to this bug.