1420107 - [meta] Crash spikes in 57

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Nevin Chen(Not active on Bugzilla)]

This bug is for tracking the crash after Fennec 57 went live

Comment 1

[Nevin Chen(Not active on Bugzilla)]

For class loading / reflection issue. These are my target to reproduce. This might be caused by proguard setting

Max teach me how to debug this
1. dex to jar release apk
2. import into a dummy project
3. minifyenable = true and basic proguard setting
4. modify Android.manifest for test build
5. run assembleDebug to see if anything missing.


16:22:07.452 [QUIET] [system.out] Note: org.mozilla.gecko.distribution.Distribution: can’t find dynamically referenced class android.os.SystemProperties
16:22:07.523 [QUIET] [system.out] Note: org.mozilla.gecko.media.GeckoPlayerFactory: can’t find dynamically referenced class org.mozilla.gecko.media.GeckoHlsPlayer
16:22:07.651 [QUIET] [system.out] Note: org.mozilla.gecko.util.PRNGFixes: can’t find dynamically referenced class org.apache.harmony.xnet.provider.jsse.NativeCrypto
16:22:07.652 [QUIET] [system.out] Note: org.mozilla.gecko.util.PRNGFixes: can’t find dynamically referenced class org.apache.harmony.xnet.provider.jsse.NativeCrypto
16
.....

I'll try to find STR to make sure they are culprit. And follow the manual to fix them if possible.
https://www.guardsquare.com/en/proguard/manual/troubleshooting

For native crashes, I think only snorp's team can help.

I think this issue is not a regression. Since we didn't change the build setting in 57.
Just because the download amount is much higher than before, so we have this symptom.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Snorp, what do you think?

Comment 3

[Nevin Chen(Not active on Bugzilla)]

Hi Kilik
I found there's an issue with release 57 apk. The app can't find this reflection class (maybe due to proguard)
See the error message when I recompiled our proguarded class again:
13:41:57.321 [QUIET] [system.out] Note: org.mozilla.gecko.media.GeckoPlayerFactory: can't find dynamically referenced class org.mozilla.gecko.media.GeckoHlsPlayer
I found you land this class in bug 1368954.  
Could you help me find a scenario when this class will be used? 
And is it working in release 57? Thank you!

Comment 4

[Nevin Chen(Not active on Bugzilla)]

tldr; I can't find a good way to fix this.

Looking at the source code here [1] for below crash

java.lang.RuntimeException: 
  at android.app.LoadedApk.makeApplication (LoadedApk.java:676)
  at android.app.ActivityThread.handleBindApplication (ActivityThread.java:6290)
  at android.app.ActivityThread.access$1800 (ActivityThread.java:221)
  at android.app.ActivityThread$H.handleMessage (ActivityThread.java:1860)
  at android.os.Handler.dispatchMessage (Handler.java:102)
  at android.os.Looper.loop (Looper.java:158)
  at android.app.ActivityThread.main (ActivityThread.java:7225)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:1230)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1120)
Caused by: java.lang.ClassNotFoundException: 
  at dalvik.system.BaseDexClassLoader.findClass (BaseDexClassLoader.java:56)
  at java.lang.ClassLoader.loadClass (ClassLoader.java:511)
  at java.lang.ClassLoader.loadClass (ClassLoader.java:469)
  at android.app.Instrumentation.newApplication (Instrumentation.java:1004)
  at android.app.LoadedApk.makeApplication (LoadedApk.java:666)

There's a chance when ClassLoader want to load a class but it's not there. 
This could only happen (in theory) when
1. We forget to keep the reflection's dynamic reference class in proguard.
2. The class have static init call that can't complete on time.
3. The crash happens when attaching baseContext to Application 


There's only very little chance for this to happen. And the crash count is low (310 times ,61 people in the past 7 days) in my two cents.
So I'll pause my investigation until ther's a clear action for me.

[1] http://androidxref.com/6.0.0_r1/xref/libcore/dalvik/src/main/java/dalvik/system/BaseDexClassLoader.java#56

Comment 5

[Nevin Chen(Not active on Bugzilla)]

Attachment: top 10 crahses

Top 10 Crashes on Play Store

Comment 6

[Nevin Chen(Not active on Bugzilla)]

Attachment: recompile.txt

Log for re-compile 57 release apk according to comment 1

Comment 7

[Nevin Chen(Not active on Bugzilla)]

Attachment: DummyApplicationForReCompile

Comment 8

[James Cheng[:JamesCheng]]

(In reply to Nevin Chen [:nechen] from comment #3)
> Hi Kilik
> I found there's an issue with release 57 apk. The app can't find this
> reflection class (maybe due to proguard)
> See the error message when I recompiled our proguarded class again:
> 13:41:57.321 [QUIET] [system.out] Note:
> org.mozilla.gecko.media.GeckoPlayerFactory: can't find dynamically
> referenced class org.mozilla.gecko.media.GeckoHlsPlayer
> I found you land this class in bug 1368954.  
> Could you help me find a scenario when this class will be used? 
> And is it working in release 57? Thank you!

Hi Nevin,

I'm not sure why we got this exception.
1. If we run into GeckoPlayerFactory code path, does the exception be raised? Even we don't use reflection way to find the class?

2. We pref off the HLS feature , so I wanna ask if there is some abnormal way that will run the java code without our expected entry point(we expected it is called by c++)?

3. We did include our java code into 57, if this is due to proguard, what should we do to prevent from proguard removing our symbols?

Thank you

Comment 9

[Nevin Chen(Not active on Bugzilla)]

Hi Nick 
Sorry for the late reply.
There wasn't much information on Play Store. The only thing I got was the back trace so I don't know which class is missing

Comment 10

[Kilik Kuo [:kikuo] (inactive)]

(In reply to Nevin Chen [:nechen] from comment #3)
> Hi Kilik
> I found there's an issue with release 57 apk. The app can't find this
> reflection class (maybe due to proguard)
> See the error message when I recompiled our proguarded class again:
> 13:41:57.321 [QUIET] [system.out] Note:
> org.mozilla.gecko.media.GeckoPlayerFactory: can't find dynamically
> referenced class org.mozilla.gecko.media.GeckoHlsPlayer
> I found you land this class in bug 1368954.  
> Could you help me find a scenario when this class will be used? 
> And is it working in release 57? Thank you!

Sorry for the late response,
We landed the HLSXXX.{cpp,h} and exoplayer2 source code in 56, and the source code are controlled by MOZ_ANDROID_HLS_SUPPORT which will be only affected if moz.configure [1] is modified. Right now, these feature & source code are only included in nighlty. So it seems to me that in release 57 we shall not hit that code path.

I don't understand why this message
'''
> org.mozilla.gecko.media.GeckoPlayerFactory: can't find dynamically
> referenced class org.mozilla.gecko.media.GeckoHlsPlayer
'''
shows up. Sorry about that.

[1] https://searchfox.org/mozilla-central/rev/7a8c667bdd2a4a32746c9862356e199627c0896d/mobile/android/moz.configure#81

Comment 11

[James Willcox (:snorp) ([e-mail]) (he/him)]

AFAICT the native crashes are not anything new and mostly seem to be OOM, which is normal. There is also some x86/arm nonsense which we are handling in bug 1370915.

Comment 12

[Nick Alexander :nalexander [he/him]]

(In reply to Nevin Chen [:nechen] from comment #9)
> Hi Nick 
> Sorry for the late reply.
> There wasn't much information on Play Store. The only thing I got was the
> back trace so I don't know which class is missing

Yeah, it's not much to go on :(  We'll have to pause that for now.

Comment 13

[Nevin Chen(Not active on Bugzilla)]

I think you can add -keep section in our proguard file :)

Comment 14

[Nevin Chen(Not active on Bugzilla)]

Hi Kilik, James
per comment 13, how do you think?

Comment 15

[Kilik Kuo [:kikuo] (inactive)]

(In reply to Nevin Chen [:nechen] from comment #14)
> Hi Kilik, James
> per comment 13, how do you think?

IIUC, according to JVM Spec [1][2], the use of reflection to initialize a class C may trigger the creation/loading of the class C when it's defined as static filed/method because that means it's included into run-time constant pool.
So JVM tries to find the class during Preparation/Resolution.

[1] https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-5.html#jvms-5.3
[2] https://docs.oracle.com/javase/specs/jvms/se7/html/jvms-5.html#jvms-5.4.2

All these 4 crash spots in Comment 1 are using Class.forName("CLASS_NAME") in a static method, making them non-static may help.
Ideas ?

Comment 16

[James Cheng[:JamesCheng]]

According to comment 15,

If JVM might do something on static filed/method/constructor, 

The only possible solution is replacing all static-used reflection code with singleton?

Comment 17

[Sylvestre Ledru [:Sylvestre]]

we shipped 57 and no real action, updating the status flags.

Comment 18

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

[triage] This seems like a meta bug that has outlived its usefulness, especially that we can look at crash stats.