Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit dfa390e0 authored by Martijn Coenen's avatar Martijn Coenen
Browse files

Serialize calls into BinderProxy. 

The BinderProxy class is not thread-safe, hence all calls into it
must be serialized. This was achieved by holding the gProxyLock in
JNI code. However, a recent change added calls into BinderProxy
from ActivityManagerService without holding that lock, causing
ConcurrentModificationExceptions.

Instead of dumping debug info from AMS, make the call directly
from JNI, so we can make sure gProxyLock is held correctly.

Also, only dump on debug builds.

Bug: 71353150
Bug: 109701487
Test: sailfish builds, boots, info gets dumped with lowered limits.
Change-Id: I446a71ce4115b9936a01a170401ef98ba3818c0b
parent dea740bd

core/java/android/os/Binder.java

+15 −13
Original line number Diff line number Diff line
@@ -137,15 +137,6 @@ public class Binder implements IBinder { 
        sTracingEnabled = false;

    }



    /**

     * Dump proxy debug information.

     *

     * @hide

     */

    public static void dumpProxyDebugInfo() {

        BinderProxy.dumpProxyDebugInfo();

    }



    /**

     * Check if binder transaction tracing is enabled.

     *
@@ -950,7 +941,8 @@ final class BinderProxy implements IBinder { 
                    // about to crash.

                    final int totalUnclearedSize = unclearedSize();

                    if (totalUnclearedSize >= CRASH_AT_SIZE) {

                        dumpProxyDebugInfo();

                        dumpProxyInterfaceCounts();

                        dumpPerUidProxyCounts();

                        Runtime.getRuntime().gc();

                        throw new AssertionError("Binder ProxyMap has too many entries: "

                                + totalSize + " (total), " + totalUnclearedSize + " (uncleared), "
@@ -1035,11 +1027,21 @@ final class BinderProxy implements IBinder { 
    private static ProxyMap sProxyMap = new ProxyMap();



    /**

      * Dump proxy debug information.

      *

      * Note: this method is not thread-safe; callers must serialize with other

      * accesses to sProxyMap, in particular {@link #getInstance(long, long)}.

      *

      * @hide

      */

    public static void dumpProxyDebugInfo() {

    private static void dumpProxyDebugInfo() {

        if (Build.IS_DEBUGGABLE) {

            sProxyMap.dumpProxyInterfaceCounts();

        sProxyMap.dumpPerUidProxyCounts();

            // Note that we don't call dumpPerUidProxyCounts(); this is because this

            // method may be called as part of the uid limit being hit, and calling

            // back into the UID tracking code would cause us to try to acquire a mutex

            // that is held during that callback.

        }

    }



    /**

core/jni/android_util_Binder.cpp

+21 −0
Original line number Diff line number Diff line
@@ -108,6 +108,7 @@ static struct binderproxy_offsets_t 
    jclass mClass;

    jmethodID mGetInstance;

    jmethodID mSendDeathNotice;

    jmethodID mDumpProxyDebugInfo;



    // Object state.

    jfieldID mNativeData;  // Field holds native pointer to BinderProxyNativeData.
@@ -1006,9 +1007,27 @@ static void android_os_BinderInternal_handleGc(JNIEnv* env, jobject clazz) 
static void android_os_BinderInternal_proxyLimitcallback(int uid)

{

    JNIEnv *env = AndroidRuntime::getJNIEnv();

    {

        // Calls into BinderProxy must be serialized

        AutoMutex _l(gProxyLock);

        env->CallStaticObjectMethod(gBinderProxyOffsets.mClass,

                                    gBinderProxyOffsets.mDumpProxyDebugInfo);

    }

    if (env->ExceptionCheck()) {

        ScopedLocalRef<jthrowable> excep(env, env->ExceptionOccurred());

        report_exception(env, excep.get(),

            "*** Uncaught exception in dumpProxyDebugInfo");

    }



    env->CallStaticVoidMethod(gBinderInternalOffsets.mClass,

                              gBinderInternalOffsets.mProxyLimitCallback,

                              uid);



    if (env->ExceptionCheck()) {

        ScopedLocalRef<jthrowable> excep(env, env->ExceptionOccurred());

        report_exception(env, excep.get(),

            "*** Uncaught exception in binderProxyLimitCallbackFromNative");

    }

}



static void android_os_BinderInternal_setBinderProxyCountEnabled(JNIEnv* env, jobject clazz,
@@ -1389,6 +1408,8 @@ static int int_register_android_os_BinderProxy(JNIEnv* env) 
            "(JJ)Landroid/os/BinderProxy;");

    gBinderProxyOffsets.mSendDeathNotice = GetStaticMethodIDOrDie(env, clazz, "sendDeathNotice",

            "(Landroid/os/IBinder$DeathRecipient;)V");

    gBinderProxyOffsets.mDumpProxyDebugInfo = GetStaticMethodIDOrDie(env, clazz, "dumpProxyDebugInfo",

            "()V");

    gBinderProxyOffsets.mNativeData = GetFieldIDOrDie(env, clazz, "mNativeData", "J");



    clazz = FindClassOrDie(env, "java/lang/Class");

services/core/java/com/android/server/am/ActivityManagerService.java

+0 −1
Original line number Diff line number Diff line
@@ -15305,7 +15305,6 @@ public class ActivityManagerService extends IActivityManager.Stub 
                        public void onLimitReached(int uid) {

                            Slog.wtf(TAG, "Uid " + uid + " sent too many Binders to uid "

                                    + Process.myUid());

                            Binder.dumpProxyDebugInfo();

                            if (uid == Process.SYSTEM_UID) {

                                Slog.i(TAG, "Skipping kill (uid is SYSTEM)");

                            } else {