Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

com.google.android.gms.measurement.internal.zzjx.onActivityCreated crash in android 7 #5948

Open
allenjia opened this issue May 9, 2024 · 13 comments

Comments

@allenjia
Copy link

allenjia commented May 9, 2024

[READ] Step 1: Are you in the right place?

Yes

[REQUIRED] Step 2: Describe your environment

  • Android Studio version: Android Studio Giraffe | 2022.3.1 Patch 4
  • Firebase Component: com.google.android.gms:play-services-measurement-base
  • Component version: 21.5.1

[REQUIRED] Step 3: Describe the problem

On Android 7.1 and lower Bundle unparceling is not thread safe.

There was a similar problem before, but it has been fixed now. The problem occurred in the firebase-messaging module. The related discussion can be seen here. #3090

In the onActivityCreated method of the class com.google.android.gms.measurement.internal.zzjx.java, the intent.getextras method is called, which may cause a crash. This issue has not been fixed yet.

#01 pc 000000000006ac60  /system/lib64/libc.so (pthread_kill+68)
#02 pc 000000000002419c  /system/lib64/libc.so (raise+28)
#03 pc 000000000001ca40  /system/lib64/libc.so (abort+56)
#04 pc 00000000000c64c8  /system/lib64/libandroid_runtime.so
#05 pc 00000000021bf1c0  /system/framework/arm64/boot-framework.oat (oatexec+9466304)
******* Java stack for JNI crash *******
android.os.Parcel.nativeAppendFrom(Parcel.java)
android.os.Parcel.appendFrom(Parcel.java:463)
android.os.BaseBundle.<init>(BaseBundle.java:164)
android.os.Bundle.<init>(Bundle.java:106)
android.content.Intent.getExtras(Intent.java:6635)
com.google.android.gms.measurement.internal.zzjx.onActivityCreated(zzjx.java:79)
com.google.android.gms.measurement.internal.AppMeasurementDynamiteService.onActivityCreated(AppMeasurementDynamiteService.java:128)
com.google.android.gms.internal.measurement.zzeo.zza(zzeo.java:11)
com.google.android.gms.internal.measurement.zzdf$zza.run(zzdf.java:12)
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1133)
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:607)
java.lang.Thread.run(Thread.java:761)

In addition, sometimes, AppMeasurementDynamiteService.class is loaded from this path "/data/user_de/0/com.google.android.gms/app_chimera/m/000000be/MeasurementDynamite.apk" through DynamiteModule, and these codes are not in our integrated firebase sdk. There is a similar set of code in MeasurementDynamite.apk, which will have the same problem. The stack is as follows:

#00 pc 0000000000026304  /system/lib64/libbinder.so (android::acquire_object(android::sp<android::ProcessState> const&, flat_binder_object const&, void const*, unsigned long*)+20)
#01 pc 00000000000282d4  /system/lib64/libbinder.so (android::Parcel::appendFrom(android::Parcel const*, unsigned long, unsigned long)+524)
#02 pc 00000000000a7a98  /system/lib64/libandroid_runtime.so
#03 pc 00000000034ee448  /data/dalvik-cache/arm64/system@framework@boot.oat (oatexec+19776584)
******* Java stack for JNI crash *******
android.os.Parcel.nativeAppendFrom(Parcel.java)
android.os.Parcel.appendFrom(Parcel.java:461)
android.os.BaseBundle.<init>(BaseBundle.java:126)
android.os.Bundle.<init>(Bundle.java:102)
android.content.Intent.getExtras(Intent.java:5694)
m.ll.onActivityCreated(:com.google.android.gms.dynamite_measurementdynamite@241616013@24.16.16 (040400-0):35)
com.google.android.gms.measurement.internal.AppMeasurementDynamiteService.onActivityCreated(:com.google.android.gms.dynamite_measurementdynamite@241616013@24.16.16 (040400-0):29)
m.cs.a(:com.google.android.gms.dynamite_measurementdynamite@241616013@24.16.16 (040400-0):114)
m.v.onTransact(:com.google.android.gms.dynamite_measurementdynamite@241616013@24.16.16 (040400-0):21)
android.os.Binder.transact(Binder.java:387)
com.google.android.gms.internal.measurement.zzbu.zzb(zzbu.java:21)
com.google.android.gms.internal.measurement.zzcw.onActivityCreated(zzcw.java:117)
com.google.android.gms.internal.measurement.zzeo.zza(zzeo.java:11)
com.google.android.gms.internal.measurement.zzdf$zza.run(zzdf.java:12)
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1113)
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:588)
java.lang.Thread.run(Thread.java:818)

Relevant Code:

com.google.android.gms.measurement.internal.zzjx.java

@google-oss-bot
Copy link
Contributor

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

@lehcar09
Copy link
Contributor

Hi @allenjia, thank you for reaching out. I tried reproducing the issue, but I was having difficulty. Could you share an runnable MCVE that can help us investigate the issue? Aside from that, is it occurring when the app is being opened or in some other way? Are you retrieving specific data from intent extras? Thanks!

@allenjia
Copy link
Author

Hi @allenjia, thank you for reaching out. I tried reproducing the issue, but I was having difficulty. Could you share an runnable MCVE that can help us investigate the issue? Aside from that, is it occurring when the app is being opened or in some other way? Are you retrieving specific data from intent extras? Thanks!

It's also difficult for me to reproduce this problem. We discovered this problem from our reporting platform. Most of the crashes occurred on Android 6 and Android 7 phones. It looked like it happened when the user tried to open some pages. At this time, the user had already opened the app and Been using it for several minutes or hours.

On low-version API Android phones, we tried a method to fix this problem, which is to obtain the mExtras field in the original BaseBundle object through reflection and use it for synchronization. It seems to be effective so far. But this method is invalid for classes dynamically loaded from MeasurementDynamite.apk.

In the BaseBundle class of Android api25 and api26, the main difference is that there is no synchronization in the copy constructor in api25. When the unparce() method of the original BaseBundle object is called in the main thread, we call Intent.getExtras() in other threads. At this time, the two will process the same Parcel data, which may cause problems. So far it looks like we are not retrieving specific data from intent extras.

@lehcar09
Copy link
Contributor

Thank you for additional details. Is there any chance you could share an MCVE to help us investigate the issue? That would help us check what intent data you're retrieving, as well as how the Firebase product is initialized.

Aside from that, could you updating to latest version 22.0.0 and see if the issue is resolved?

@google-oss-bot
Copy link
Contributor

Hey @allenjia. We need more information to resolve this issue but there hasn't been an update in 5 weekdays. I'm marking the issue as stale and if there are no new updates in the next 5 days I will close it automatically.

If you have more information that will help us get to the bottom of this, just add a comment!

@allenjia
Copy link
Author

Thank you for additional details. Is there any chance you could share an MCVE to help us investigate the issue? That would help us check what intent data you're retrieving, as well as how the Firebase product is initialized.

Aside from that, could you updating to latest version 22.0.0 and see if the issue is resolved?

I tried many times but couldn't reproduce this problem, so I can't provide an MCVE yet. After we integrated the firebase sdk, we only called a few methods about analytics, just including setAnalyticsCollectionEnabled, setConsent and setUserId.

We will try to update to the latest version next. But most crashes belong to the second type, that is, the classes are loaded from MeasurementDynamite.apk, which is part of Google Play Services, so I think It can't be solved until the Google Play Servicesis upgraded.

As mentioned above, we have fixed the first type of problem which classes are loaded from firebase sdk by using reflection and synchronization.

    public final zzcu zza(Context context, boolean z) {
        try {
            return zzct.asInterface(DynamiteModule.load(context, DynamiteModule.PREFER_HIGHEST_OR_LOCAL_VERSION, ModuleDescriptor.MODULE_ID).instantiate("com.google.android.gms.measurement.internal.AppMeasurementDynamiteService"));
        } catch (DynamiteModule.LoadingException e) {
            zza((Exception) e, true, false);
            return null;
        }
    }

For the second type of problem which classes are loaded from MeasurementDynamite.apk, we find some other details. There is a param of "DynamiteModule.PREFER_HIGHEST_OR_LOCAL_VERSION" in com.google.android.gms.internal.measurement.zzdf.zza(). If we change it to "DynamiteModule.PREFER_LOCAL", the releated class, such as AppMeasurementDynamiteService.class, will be loaded from the firebase sdk instead of from the MeasurementDynamite.apk.

Can we fix the second type of problem by change the param from "DynamiteModule.PREFER_HIGHEST_OR_LOCAL_VERSION" to "DynamiteModule.PREFER_LOCAL"? I think this will be effective because the problem in firebase sdk has been fixed by reflection and synchronization. Will this approach cause other potential problems?

@lehcar09
Copy link
Contributor

Just wanted to check in, does the issue persist on the latest version of Firebase Analytics 22.0.0?

@google-oss-bot
Copy link
Contributor

Hey @allenjia. We need more information to resolve this issue but there hasn't been an update in 5 weekdays. I'm marking the issue as stale and if there are no new updates in the next 5 days I will close it automatically.

If you have more information that will help us get to the bottom of this, just add a comment!

@allenjia
Copy link
Author

allenjia commented Jun 6, 2024

Hi @allenjia, thank you for reaching out. I tried reproducing the issue, but I was having difficulty. Could you share an runnable MCVE that can help us investigate the issue? Aside from that, is it occurring when the app is being opened or in some other way? Are you retrieving specific data from intent extras? Thanks!

    public void run() {
        Bundle bundleMap = new Bundle();
        for (int k = 0; k < 1000; k++) {
            bundleMap.putString("test" + k, "hello world " + k);
        }
        new Thread(() -> {
            for (int i = 0; i < 100000000; i++) {
                Parcel parcel = Parcel.obtain();
                bundleMap.writeToParcel(parcel, 0);
                parcel.setDataPosition(0);
                Bundle bundleParcel = new Bundle();
                bundleParcel.readFromParcel(parcel);
                parcel.recycle();
                new Thread(() -> {
                    Thread.yield();
                    try {
                        new Bundle(bundleParcel);
                    } catch (RuntimeException e) {
                        Log.e("testtesttest", "exception ", e);
                    }
                }).start();
                new Thread(() -> {
                    Thread.yield();
                    bundleParcel.getString("test10");
                }).start();
                Log.e("testtesttest", "run: " + i);
            }
        }).start();
    }

I reproduced this problem with the above codes on an Android 5.1.1 phone.

java.lang.NullPointerException: Attempt to invoke virtual method 'int android.os.Parcel.dataSize()' on a null object reference
android.os.BaseBundle.<init>(BaseBundle.java:126)
android.os.Bundle.<init>(Bundle.java:102)
......
#00 pc 0000000000013b98  libc.so (memcpy+336)
#01 pc 000000000002d4f0  libbinder.so (android::Parcel::appendFrom(android::Parcel const*, unsigned long, unsigned long)+332)
#02 pc 00000000000d01c0  libandroid_runtime.so
#03 pc 00000000747d6df4  <unknown>
******* Java stack for JNI crash *******
android.os.Parcel.nativeAppendFrom(Parcel.java)
android.os.Parcel.appendFrom(Parcel.java:446)
android.os.BaseBundle.<init>(BaseBundle.java:126)
android.os.Bundle.<init>(Bundle.java:102)
......

There are two main types of errors, including NullPointerException and JNI crash. The NullPointerException is easy to reproduce after hundreds or thousands of attempts, but the JNI crash may requires tens of thousands of attempts to reproduce.

In class zzjx of Firebase Sdk, the RuntimeException is caught, just like I did in the demo above, so our app crashes are basically JNI crash.

@lehcar09
Copy link
Contributor

lehcar09 commented Jun 12, 2024

Thank you for the additional details @allenjia. I was able to reproduce the issue with the code snippet you shared with me. I'll inform our engineers and see what we can do here.

In the meantime, could you share the contents of your gradle files?

@allenjia
Copy link
Author

allenjia commented Jun 14, 2024

In the meantime, could you share the contents of your gradle files?

com.google.android.gms:play-services-measurement-api:21.5.1
com.google.android.gms:play-services-measurement-base:21.5.1
com.google.android.gms:play-services-measurement-impl:21.5.1
com.google.android.gms:play-services-measurement-sdk-api:21.5.1
com.google.android.gms:play-services-measurement-sdk:21.5.1
com.google.android.gms:play-services-measurement:21.5.1
com.google.firebase:firebase-analytics:21.5.1

We integrated these modules of firebase analytics. After we upgrade these modules to version 21.5.1, the following modules also need to be upgraded according to their dependencies.

androidx.privacysandbox.ads:ads-adservices:1.0.0-beta05 
androidx.privacysandbox.ads:ads-adservices-java:1.0.0-beta05
com.google.firebase:firebase-common:20.4.2
com.google.firebase:firebase-common-ktx:20.4.2
com.google.firebase:firebase-components:17.1.5
org.jetbrains.kotlinx:kotlinx-coroutines-android:1.7.1
org.jetbrains.kotlinx:kotlinx-coroutines-bom:1.7.1
org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm:1.7.1
org.jetbrains.kotlinx:kotlinx-coroutines-core:1.7.1
org.jetbrains.kotlinx:kotlinx-coroutines-play-services:1.7.1
org.jetbrains:annotations:23.0.0

We cannot provide the files directly because there is some sensitive information. Is the above content sufficient? What other information is needed? Thanks! @lehcar09

@lehcar09
Copy link
Contributor

Thanks for the additional details @allenjia. These details are sufficient. We are tracking the issue internally in b/347706599. I'll keep you posted for updates.

@sahibdeep-singh
Copy link

@lehcar09 Any update on this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants