880171 - java.lang.NullPointerException: at org.mozilla.gecko.background.healthreport.EnvironmentBuilder.getStorage(EnvironmentBuilder.java)

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Scoobidiver (away)]

There's one crash in 24.0a1/20130604: bp-9137613a-f142-4bc8-9ef9-e93712130605.

java.lang.NullPointerException
	at org.mozilla.gecko.background.healthreport.EnvironmentBuilder.getStorage(EnvironmentBuilder.java:34)
	at org.mozilla.gecko.health.BrowserHealthRecorder.<init>(BrowserHealthRecorder.java:106)
	at org.mozilla.gecko.GeckoApp$14.run(GeckoApp.java:1218)
	at android.os.Handler.handleCallback(Handler.java:605)
	at android.os.Handler.dispatchMessage(Handler.java:92)
	at android.os.Looper.loop(Looper.java:154)
	at org.mozilla.gecko.util.GeckoBackgroundThread.run(GeckoBackgroundThread.java:32)

More reports at:
https://crash-stats.mozilla.com/report/list?signature=java.lang.NullPointerException%3A+at+org.mozilla.gecko.background.healthreport.EnvironmentBuilder.getStorage%28EnvironmentBuilder.java%29

Comment 1

[Richard Newman [:rnewman]]

This sounds like the equivalent of that npe fetching the passwords provider. The one that doesn't make sense.

Comment 2

[Richard Newman [:rnewman]]

This exception means that acquireContentProviderClient returned null for our authority. That should never happen; if it does, it should happen for *every* install of that APK.

This is exactly the same problem as Bug 745837.

And hey, check out the crap in the crash report:

https://crash-stats.mozilla.com/report/index/9137613a-f142-4bc8-9ef9-e93712130605

Running Android 0.0.0 on an HTC HTC Desire S. Right. Custom ROM that doesn't work right? Disabled permissions?

We can add a guard around this, but that's the point of defensive coding where you simply don't believe that any API call will work at all. Android has driven us here.

Comment 3

[Richard Newman [:rnewman]]

Also, I'm hesitant to 'fix' this, because the fix will cause us to have no errors reported, instead silently failing to init FHR. Let's leave this for a little while, see if it was an isolated incident.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

The crash report seems valid. There are a lot of crashes reported with "HTC HTC Desire S" as the device and 0.0.0 as the Linux OS version. The Android version appears to actually be 4.0.4.

The crash happens at http://hg.mozilla.org/mozilla-central/file/7e3a4ebcf067/mobile/android/base/background/healthreport/EnvironmentBuilder.java#l34 which means "pr" (returned from getLocalContentProvider() just above) is null. This seems to be allowed by the javadoc for getLocalContentProvider(), although I guess it might violate the expectations in that area of the code, I'm not sure.

Comment 5

[Richard Newman [:rnewman]]

You're right; I misread the line number.

Same kind of invariant failing, though: it means there's no CP running in the same thread for that authority, which should not be the case.

Poking around in the Android source now.

Comment 6

[Richard Newman [:rnewman]]

I've been turning this over in my head. The only way I can see this happening is if the CP is first instantiated in a different process to the crashing GeckoApp's thread. 

Android ensures that there's only one of the CP, and it lives in our main process (because we don't specify otherwise), regardless of who spawned it. So this implies a new process has instantiated a BrowserHealthRecorder or GeckoApp. 

Is that possible? Can an app launch a GeckoApp activity in its own process in current Nightly? 

Either that, or this is a weird Android bug. (Also to investigate: perhaps the passwords process is alive, and somehow claimed our CP?) 

I figure Brad might know the answer to that last question...

Comment 7

[Richard Newman [:rnewman]]

Band-aid patch for this yields the following behavior:

I/GeckoApp( 5958): Creating BrowserHealthRecorder.
D/GeckoHealthRec( 5958): Initializing. Dispatcher is org.mozilla.gecko.util.EventDispatcher@42862170
E/GeckoLogger( 5958): fennec_rnewman :: GeckoEnvBuilder :: Unable to retrieve local content provider. Running in a different process?
D/GeckoHealthRec( 5958): Initializing profile cache.
I/GeckoLogger( 5958): fennec_rnewman :: GeckoProfileInfo :: Restoring ProfileInformationCache from file.
D/GeckoHealthRec( 5958): Successfully restored state. Initializing storage.
D/GeckoHealthRec( 5958): Done initializing profile cache. Beginning storage init.
W/GeckoHealthRec( 5958): Storage is null during init; shutting down?
D/GeckoSessInfo( 5958): Recording start of session: 1370567957195
...
D/GeckoHealthRec( 5958): Recording session end: P
E/GeckoHealthRec( 5958): Attempted to record session end without initialized recorder.
I/GeckoHealthRec( 7440): Closing incompletely initialized BrowserHealthRecorder.
W/GeckoEventDispatcher( 7440): unregisterEventListener: event 'Addons:All' has no listeners
W/GeckoEventDispatcher( 7440): unregisterEventListener: event 'Addons:Change' has no listeners
W/GeckoEventDispatcher( 7440): unregisterEventListener: event 'Pref:Change' has no listeners
W/GeckoEventDispatcher( 7440): unregisterEventListener: event 'Search:Keyword' has no listeners
W/GeckoEventDispatcher( 7440): unregisterEventListener: event 'Search:Event' has no listeners


That is: we'll quietly log a few failures, and otherwise won't interfere.

I'll prep this for landing, but I want to see if we get any more crashes, so I won't land it just yet.

Comment 8

[Richard Newman [:rnewman]]

Attachment: Part 1: return null if unable to retrieve storage in EnvironmentBuilder. (git)

Comment 9

[Richard Newman [:rnewman]]

Attachment: Part 2: handle failure to retrieve storage from EnvironmentBuilder.

Comment 10

[Richard Newman [:rnewman]]

https://github.com/mozilla-services/android-sync/pull/318 is Part 1.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Richard Newman [:rnewman] from comment #6)

> Android ensures that there's only one of the CP, and it lives in our main
> process (because we don't specify otherwise), regardless of who spawned it.
> So this implies a new process has instantiated a BrowserHealthRecorder or
> GeckoApp. 
> 
> Is that possible? Can an app launch a GeckoApp activity in its own process
> in current Nightly? 

The only thing that comes to my mind is WebApps. They do run in a different process than the Firefox browser. But I don't think this is what you are referring to. I can't think of any other processes instantiating GeckoApp.

Comment 12

[Richard Newman [:rnewman]]

https://hg.mozilla.org/services/services-central/rev/1782e033c17a
https://hg.mozilla.org/services/services-central/rev/2d3c9e3daa76

Comment 13

[Richard Newman [:rnewman]]

https://hg.mozilla.org/mozilla-central/rev/1782e033c17a
https://hg.mozilla.org/mozilla-central/rev/2d3c9e3daa76

Comment 14

[Richard Newman [:rnewman]]

Comment on attachment 759533 [details] [diff] [review]
Part 1: return null if unable to retrieve storage in EnvironmentBuilder. (git)

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Affects code in Bug 868449, but caused by... Android.

User impact if declined: 
  One startup crash on Nightly since all this landed; it's possible there could be more.

Testing completed (on m-c, etc.): 
  Bake time.

Risk to taking this patch (and alternatives if risky): 
  None.

String or IDL/UUID changes made by this patch:
  None.

Comment 15

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 759533 [details] [diff] [review]
Part 1: return null if unable to retrieve storage in EnvironmentBuilder. (git)

Approving as part of the body of work needed for FHR on FF23 Android.

Comment 16

[Richard Newman [:rnewman]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/659227d5485c
https://hg.mozilla.org/releases/mozilla-aurora/rev/54b272e9542f

Comment 17

[Richard Newman [:rnewman]]

You beat me to it! m-cMerge doesn't set status flags unless a bug is tracking.