1155994 - TabQueue : Attempt to read from field 'java.lang.String android.content.Intent.mAction' on a null object reference

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Martyn Haigh (:mhaigh)]

I randomly get Tab Queue toasts popping up when I've not performed any action to initiate it.  I clicked "Open Now" on one and captured this error message, we should add some defensive code to prevent the toast from showing before it gets a chance to show the toast:


E/AndroidRuntime(14635): FATAL EXCEPTION: main
E/AndroidRuntime(14635): Process: org.mozilla.fennec_martyn, PID: 14635
E/AndroidRuntime(14635): java.lang.NullPointerException: Attempt to read from field 'java.lang.String android.content.Intent.mAction' on a null object reference
E/AndroidRuntime(14635): 	at android.content.Intent.<init>(Intent.java:4060)
E/AndroidRuntime(14635): 	at org.mozilla.gecko.tabqueue.TabQueueService$2.onClick(TabQueueService.java:139)
E/AndroidRuntime(14635): 	at android.view.View.performClick(View.java:4780)
E/AndroidRuntime(14635): 	at android.view.View$PerformClick.run(View.java:19866)
E/AndroidRuntime(14635): 	at android.os.Handler.handleCallback(Handler.java:739)
E/AndroidRuntime(14635): 	at android.os.Handler.dispatchMessage(Handler.java:95)
E/AndroidRuntime(14635): 	at android.os.Looper.loop(Looper.java:135)
E/AndroidRuntime(14635): 	at android.app.ActivityThread.main(ActivityThread.java:5254)
E/AndroidRuntime(14635): 	at java.lang.reflect.Method.invoke(Native Method)
E/AndroidRuntime(14635): 	at java.lang.reflect.Method.invoke(Method.java:372)
E/AndroidRuntime(14635): 	at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:903)
E/AndroidRuntime(14635): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:698)

Comment 1

[Richard Newman [:rnewman]]

Sounds like Android is resuming your service after it was killed. It'll ostensibly redeliver the last intent, which might be null. It seems to do this sometimes regardless of the manifest configuration for the service.

See Bug 1025937 and friends.

A null check and short-circuit is appropriate here.

Comment 2

[Richard Newman [:rnewman]]

Sorry, to clarify: that last comment is about the cause of the toast popping up in the first place, which is likely to be service restart. Belt-and-braces, though: if it does pop up, tapping it shouldn't cause a crash.

Comment 3

[Martyn Haigh (:mhaigh)]

Yeah - this is weird, the service is being started from somewhere other than the TabQueueDispatcher which is the only place in our code which starts this service, and the Service is not exported (by virtue of having no filters defined in the manifest).  

I can't see any pattern to when this happens but have put in some logging and have been running locally and the service is being started with a null intent.  A fix is easy, but I've no idea why this is happening.

Comment 4

[Richard Newman [:rnewman]]

http://developer.android.com/reference/android/app/Service.html#START_STICKY

Comment 5

[Martyn Haigh (:mhaigh)]

Ah - I see the problem.  We currently incorrectly return START_FLAG_REDELIVERY, whereas we should actually be returning START_REDELIVER_INTENT.  START_FLAG_REDELIVERY relates to the flag set in the flags argument in the onStartCommand, not the value returned from onStartCommand.  START_FLAG_REDELIVERY == 1 && START_STICKY == 1, hence we're seeing the behaviour of START_STICKY.

Comment 6

[Martyn Haigh (:mhaigh)]

Attachment: Fix null intent error in TabQueueService

Comment 7

[Richard Newman [:rnewman]]

Comment on attachment 8594431 [details] [diff] [review]
Fix null intent error in TabQueueService

Review of attachment 8594431 [details] [diff] [review]:
-----------------------------------------------------------------

This is in theory the right fix; you do stop the service when the toast has timed out, so this is the response we want to make sure we get redelivered if the service dies during processing. No nulls.

But that makes your observation in Comment 0 a little weird: why is the service being restarted in the first place? That implies that it's not successfully calling stopService, and Android will eventually restart it.

Let's land this, but keep an eye out for unexpected duplicate tabs/toasts/whatever, because there might still be something wrong earlier in the process.

Comment 8

[Martyn Haigh (:mhaigh)]

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

Comment 9

[Martyn Haigh (:mhaigh)]

rnewman: yeah, have found the cause - we're not explicitly stopping the service when the Open Now button is pressed.  have created bug 1156093 to track

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2a3f8eefee8e