Closed Bug 1289006 Opened 3 years ago Closed 3 years ago

Crash in java.lang.NullPointerException: storage == null at java.util.Arrays$ArrayList.<init>(Arrays.java)

Categories

(Firefox for Android :: General, defect, P2, critical)

Unspecified
Android
defect

Tracking

()

RESOLVED FIXED
Firefox 50
Tracking Status
firefox50 --- fixed

People

(Reporter: njn, Assigned: mcomella)

Details

(Keywords: crash)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-d28d059e-44ab-4fe0-904f-c27292160724.
=============================================================

This crash first appeared in Fennec Nightly 20160621030208. Since then it has occurred 21 times. The Java stack trace is this:

> java.lang.NullPointerException: storage == null
>   at java.util.Arrays$ArrayList.<init>(Arrays.java:38)
>   at java.util.Arrays.asList(Arrays.java:155)
>   at org.mozilla.gecko.telemetry.stores.TelemetryJSONFilePingStore.getAllPings(TelemetryJSONFilePingStore.java:132)
>   at org.mozilla.gecko.telemetry.stores.TelemetryJSONFilePingStore.getAllPings(TelemetryJSONFilePingStore.java:64)
>   at org.mozilla.gecko.telemetry.TelemetryUploadService.uploadPendingPingsFromStore(TelemetryUploadService.java:98)
>   at org.mozilla.gecko.telemetry.TelemetryUploadService.onHandleIntent(TelemetryUploadService.java:81)
>   at android.app.IntentService$ServiceHandler.handleMessage(IntentService.java:66)
>   at android.os.Handler.dispatchMessage(Handler.java:102)
>   at android.os.Looper.loop(Looper.java:148)
>   at android.os.HandlerThread.run(HandlerThread.java:61)

Michael, can you please investigate?
Flags: needinfo?(michael.l.comella)
Priority: -- → P2
I am the guy responsible for most of the crash reports. If you need anything, ask me.
Digging in, the exception is thrown if the argument to ArrayList.<init> is null:
 http://androidxref.com/4.4.4_r1/xref/libcore/luni/src/main/java/java/util/Arrays.java#36

Looking at where the arg comes from, we see Arrays.asList:
 http://androidxref.com/4.4.4_r1/xref/libcore/luni/src/main/java/java/util/Arrays.java#154

Who gets its argument from the ping store, which calls listFiles:
 http://androidxref.com/4.4.4_r1/xref/libcore/luni/src/main/java/java/io/File.java#806

Which calls to list:
 http://androidxref.com/4.4.4_r1/xref/libcore/luni/src/main/java/java/io/File.java#766

And returns null when the native list method returns null or the filename filter is null. Since the filename filter is null, I assume the native list method is returning null, which I imagine might happen when the file in File.listFiles either does not exist or is a directory.

Quick fix is to return gracefully if the output is null.

In the interest of trying to figure out why this happened (because it's unexpected)...

(In reply to tobbi.bugs from comment #1)
> I am the guy responsible for most of the crash reports. If you need
> anything, ask me.

Do you have any special configuration for your phone/Firefox? Perhaps your disk is full or Firefox was not allowed to freely write to its data directory?
Assignee: nobody → michael.l.comella
Flags: needinfo?(michael.l.comella) → needinfo?(tobbi.bugs)
This changeset will correct the crash we're seeing in the bug.

The docs support that File.listFiles can return null.

Review commit: https://reviewboard.mozilla.org/r/66942/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66942/
Attachment #8774501 - Flags: review?(gkruglov)
Attachment #8774502 - Flags: review?(gkruglov)
I expect the crashes occurred because one of the following:
  * the store already existed as a file
  * the store was a directory that did not have the appropriate permissions

In the telemetry code, none of the cases above should happen so I assert that
they never do.

If the crashes did occur for these reasons, the user will unfortunately
continue to crash but at least we'll know where our assumptions are going wrong.

I originally intended to write a regression test for listFiles returning null
but it requires the application code to be modified in non-trivial ways (e.g.
accessor methods we might forget to use) so I decided against it.

Review commit: https://reviewboard.mozilla.org/r/66944/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66944/
Attachment #8774501 - Flags: review?(gkruglov) → review+
Comment on attachment 8774501 [details]
Bug 1289006 - Return gracefully if listFiles returns null in telemetry store.

https://reviewboard.mozilla.org/r/66942/#review63812
Comment on attachment 8774502 [details]
Bug 1289006 - Add code to assert potential crashing case never happens & tests.

https://reviewboard.mozilla.org/r/66944/#review63814

Strange stuff; hopefully the added logging will help narrow this down.
Attachment #8774502 - Flags: review?(gkruglov) → review+
https://hg.mozilla.org/integration/fx-team/rev/d5403a0e1c779912d79e14f7cd5d22e978653f89
Bug 1289006 - Return gracefully if listFiles returns null in telemetry store. r=grisha

https://hg.mozilla.org/integration/fx-team/rev/5a8b18f503baa996bcba9ea249e1762b7e536211
Bug 1289006 - Add code to assert potential crashing case never happens & tests. r=grisha
Pushed by michael.l.comella@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/d5403a0e1c77
Return gracefully if listFiles returns null in telemetry store. r=grisha
https://hg.mozilla.org/integration/fx-team/rev/5a8b18f503ba
Add code to assert potential crashing case never happens & tests. r=grisha
Thank you for the fast fix.
https://hg.mozilla.org/mozilla-central/rev/d5403a0e1c77
https://hg.mozilla.org/mozilla-central/rev/5a8b18f503ba
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Flags: needinfo?(tobbi.bugs)
You need to log in before you can comment on or make changes to this bug.