Apps-android-commons: [Urgent] Crashes in v2.6.5

[misaochan]

We are getting a lot of crashes in v2.6.5 production, with a crash rate of 4.27%, more than in previous versions. This has unfortunately led to a lot of uninstalls. :( The majority of the crashes seem to be related to Dagger. I have pasted them below:

• 11 reports, 8 users using Android 8.0 and 8.1:

Pixel 2 XL (taimen) | 3 | 27.3%
Nexus 6P (angler) | 3 | 27.3%
Pixel (sailfish) | 2 | 18.2%
Pixel XL (marlin) | 1 | 9.1%
Others | 2 | 18.2%

java.lang.RuntimeException: 
  at android.app.ActivityThread.handleCreateService (ActivityThread.java:3349)
  at android.app.ActivityThread.-wrap4 (Unknown Source)
  at android.app.ActivityThread$H.handleMessage (ActivityThread.java:1677)
  at android.os.Handler.dispatchMessage (Handler.java:106)
  at android.os.Looper.loop (Looper.java:164)
  at android.app.ActivityThread.main (ActivityThread.java:6494)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:438)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:807)
Caused by: java.lang.RuntimeException: 
  at dagger.android.AndroidInjection.inject (AndroidInjection.java:134)
  at dagger.android.DaggerService.onCreate (DaggerService.java:27)
  at fr.free.nrw.commons.HandlerService.onCreate (HandlerService.java:55)
  at fr.free.nrw.commons.upload.UploadService.onCreate (UploadService.java:122)
  at android.app.ActivityThread.handleCreateService (ActivityThread.java:3339)

• 21 reports, 7 users using Android 7.0 and 7.1:

Moto G(4) (athene) | 5 | 33.3%
Moto G (5) Plus (potter_n) | 4 | 26.7%
Moto G (5S) (montana) | 2 | 13.3%
Aquaris U (chaozu) | 2 | 13.3%

java.lang.RuntimeException: 
  at android.app.ActivityThread.handleCreateService (ActivityThread.java:3544)
  at android.app.ActivityThread.-wrap6 (ActivityThread.java)
  at android.app.ActivityThread$H.handleMessage (ActivityThread.java:1732)
  at android.os.Handler.dispatchMessage (Handler.java:102)
  at android.os.Looper.loop (Looper.java:154)
  at android.app.ActivityThread.main (ActivityThread.java:6776)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:1496)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1386)
Caused by: java.lang.RuntimeException: 
  at dagger.android.AndroidInjection.inject (AndroidInjection.java:134)
  at dagger.android.DaggerService.onCreate (DaggerService.java:27)
  at fr.free.nrw.commons.HandlerService.onCreate (HandlerService.java:55)
  at fr.free.nrw.commons.upload.UploadService.onCreate (UploadService.java:122)
  at android.app.ActivityThread.handleCreateService (ActivityThread.java:3534)

• 5 reports, 2 users using Android 7.1, both using RedMi phones.

java.lang.RuntimeException: 
  at fr.free.nrw.commons.contributions.ContributionsActivity$1.onServiceDisconnected (ContributionsActivity.java:97)
  at android.app.LoadedApk$ServiceDispatcher.doConnected (LoadedApk.java:1457)
  at android.app.LoadedApk$ServiceDispatcher$RunConnection.run (LoadedApk.java:1489)
  at android.os.Handler.handleCallback (Handler.java:754)
  at android.os.Handler.dispatchMessage (Handler.java:95)
  at android.os.Looper.loop (Looper.java:163)
  at android.app.ActivityThread.main (ActivityThread.java:6361)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:904)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:794)

• 10 reports, 5 users using Android 7.0 and 7.1:

Galaxy J7 Pro (j7y17lte) | 2 | 25.0%
Galaxy J5(2016) (j5xnlte) | 2 | 25.0%
Galaxy S7 (heroqltespr) | 2 | 25.0%
Galaxy S8 (dreamlte) | 2 | 25.0%

java.lang.RuntimeException: 
  at android.app.ActivityThread.handleCreateService (ActivityThread.java:3193)
  at android.app.ActivityThread.-wrap5 (ActivityThread.java)
  at android.app.ActivityThread$H.handleMessage (ActivityThread.java:1563)
  at android.os.Handler.dispatchMessage (Handler.java:102)
  at android.os.Looper.loop (Looper.java:154)
  at android.app.ActivityThread.main (ActivityThread.java:6123)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:867)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:757)
Caused by: java.lang.RuntimeException: 
  at dagger.android.AndroidInjection.inject (AndroidInjection.java:134)
  at dagger.android.DaggerService.onCreate (DaggerService.java:27)
  at fr.free.nrw.commons.HandlerService.onCreate (HandlerService.java:55)
  at fr.free.nrw.commons.upload.UploadService.onCreate (UploadService.java:122)
  at android.app.ActivityThread.handleCreateService (ActivityThread.java:3183)

[misaochan]

Hm, seems like 3 of the crash clusters stem from the same line of code: fr.free.nrw.commons.HandlerService.onCreate (HandlerService.java:55)

[misaochan]

Crash rate has gone up to 5.43% with the Dagger-related crash. :/ @maskaravivek @psh , any thoughts on how we can fix this? Not sure why we are experiencing so many issues with Dagger.

[psh]

It looks like all of these stack traces boil down to the same root cause.

// this should never happen
throw new RuntimeException("UploadService died but the rest of the process did not!");

Is, in fact, happening & that exception is a symptom. The real, troublesome line in question - AndroidInjection.inject() line 134 - says that the application associated with the UploadService doesn't implement HasServiceInjector and yet our commons application class extends DaggerApplication which clearly does.

Tracking backward, it looks like MultipleShareActivity interacts with the service - is it possible that we are being invoked from the "share" option in the Android 8.0 / 8.1 photos app, and as such are associated with their application class in that case and not our own?

Looking at the injected dependencies in the UploadService there isn't a huge need to manage them as Singletons - we could change how injection is done in that service (more traditional Dagger without the AndroidInjection) or possibly make the class a touch more standalone and instantiate the 3 dependencies directly. Thoughts?

[misaochan]

Thanks for the thoughts @psh !

Tracking backward, it looks like MultipleShareActivity interacts with the service - is it possible that we are being invoked from the "share" option in the Android 8.0 / 8.1 photos app, and as such are associated with their application class in that case and not our own?

Yeah, very plausible. Initiating the upload via "Share" is possible for all Android versions I think, and is currently the only way to access MultipleShareActivity. We really need to overhaul that in the future, since MultipleShareActivity always gets left behind in any of our changes (and personally I always forget to test using it).

Looking at the injected dependencies in the UploadService there isn't a huge need to manage them as Singletons - we could change how injection is done in that service (more traditional Dagger without the AndroidInjection) or possibly make the class a touch more standalone and instantiate the 3 dependencies directly. Thoughts?

I'm admittedly not very familiar with how Dagger works, but either of the above options sound good to me.

[psh]

Doing some poking around, I ran across https://stackoverflow.com/questions/46784685/runtimeexception-with-dagger-2-on-android-7-0-and-samsung-devices

[maskaravivek]

@psh This could very well be our source of crashes. I will refactor the usage of these dagger classes.

[misaochan]

Thanks for the input guys! I've updated the opening post with the recent number of crashes as well as the devices that recorded the crash (since it appears to be a contributing factor). Annoyingly, despite having a Samsung Galaxy S7 running 7.0... I still can't reproduce my crash cluster, lol.

[nicolas-raoul]

How about releasing a beta including the latest changes?
(Unfortunately I can't reproduce the crash though)

[misaochan]

@nicolas-raoul I think @maskaravivek is currently working on a Dagger refactor based on the advice received here. Since it will likely only take a few more days, I figured we could wait for that and then do a beta release with that.

[neslihanturan]

We hope this is solved currently.