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

Commit eed5b5a3 authored by Makoto Onuki's avatar Makoto Onuki
Browse files

Add basic inversion lock detection to DPMS. 

For now enable it on ENG builds only.
(I'll change the condition in master so I'll get WTFs from qt-release devices
too.)

This will detect calling into DPMS with the following locks held:

APP_OPS
POWER
USER
PACKAGES
STORAGE
WINDOW
ACTIVITY
DPMS

On marlin-eng pi-dev, each guard() takes ~25us.
    LockGuard.guard(): count=7246, total=175.1ms, avg=0.024ms

Used the following command to ensure all locks are replaced.
$ grep synchronized /android/pi-dev/frameworks/base/services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java | sed -e 's/  *//' | uniq

Bug: 74553426
Test: Manual test with an intentional lock inversion.
Change-Id: Id59d562d7c275b6ea127a211284496f5d64f9f93
parent a926126a

services/core/java/com/android/server/LockGuard.java

+49 −11
Original line number Diff line number Diff line
@@ -16,10 +16,14 @@ 


package com.android.server;



import android.annotation.Nullable;

import android.text.TextUtils;

import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.Slog;



import com.android.internal.os.BackgroundThread;



import java.io.FileDescriptor;

import java.io.PrintWriter;
@@ -60,8 +64,6 @@ import java.io.PrintWriter; 
public class LockGuard {

    private static final String TAG = "LockGuard";



    public static final boolean ENABLED = false;



    /**

     * Well-known locks ordered by fixed index. Locks with a specific index

     * should never be acquired while holding a lock of a lower index.
@@ -73,8 +75,9 @@ public class LockGuard { 
    public static final int INDEX_STORAGE = 4;

    public static final int INDEX_WINDOW = 5;

    public static final int INDEX_ACTIVITY = 6;

    public static final int INDEX_DPMS = 7;



    private static Object[] sKnownFixed = new Object[INDEX_ACTIVITY + 1];

    private static Object[] sKnownFixed = new Object[INDEX_DPMS + 1];



    private static ArrayMap<Object, LockInfo> sKnown = new ArrayMap<>(0, true);
@@ -84,6 +87,9 @@ public class LockGuard { 


        /** Child locks that can be acquired while this lock is already held */

        public ArraySet<Object> children = new ArraySet<>(0, true);



        /** If true, do wtf instead of a warning log. */

        public boolean doWtf;

    }



    private static LockInfo findOrCreateLockInfo(Object lock) {
@@ -114,9 +120,9 @@ public class LockGuard { 
            if (child == null) continue;



            if (Thread.holdsLock(child)) {

                Slog.w(TAG, "Calling thread " + Thread.currentThread().getName() + " is holding "

                      + lockToString(child) + " while trying to acquire "

                      + lockToString(lock), new Throwable());

                doLog(lock, "Calling thread " + Thread.currentThread().getName()

                        + " is holding " + lockToString(child) + " while trying to acquire "

                        + lockToString(lock));

                triggered = true;

            }

        }
@@ -145,13 +151,30 @@ public class LockGuard { 
        for (int i = 0; i < index; i++) {

            final Object lock = sKnownFixed[i];

            if (lock != null && Thread.holdsLock(lock)) {

                Slog.w(TAG, "Calling thread " + Thread.currentThread().getName() + " is holding "

                        + lockToString(i) + " while trying to acquire "

                        + lockToString(index), new Throwable());



                // Note in this case sKnownFixed may not contain a lock at the given index,

                // which is okay and in that case we just don't do a WTF.

                final Object targetMayBeNull = sKnownFixed[index];

                doLog(targetMayBeNull, "Calling thread " + Thread.currentThread().getName()

                        + " is holding " + lockToString(i) + " while trying to acquire "

                        + lockToString(index));

            }

        }

    }



    private static void doLog(@Nullable Object lock, String message) {

        if (lock != null && findOrCreateLockInfo(lock).doWtf) {



            // Don't want to call into ActivityManager with any lock held, so let's just call it

            // from a new thread. We don't want to use known threads (e.g. BackgroundThread) either

            // because they may be stuck too.

            final Throwable stackTrace = new RuntimeException(message);

            new Thread(() -> Slog.wtf(TAG, stackTrace)).start();

            return;

        }

        Slog.w(TAG, message, new Throwable());

    }



    /**

     * Report the given lock with a well-known label.

     */
@@ -165,19 +188,33 @@ public class LockGuard { 
     * Report the given lock with a well-known index.

     */

    public static Object installLock(Object lock, int index) {

        return installLock(lock, index, /*doWtf=*/ false);

    }



    /**

     * Report the given lock with a well-known index.

     */

    public static Object installLock(Object lock, int index, boolean doWtf) {

        sKnownFixed[index] = lock;

        final LockInfo info = findOrCreateLockInfo(lock);

        info.doWtf = doWtf;

        info.label = "Lock-" + lockToString(index);

        return lock;

    }



    public static Object installNewLock(int index) {

        return installNewLock(index, /*doWtf=*/ false);

    }



    public static Object installNewLock(int index, boolean doWtf) {

        final Object lock = new Object();

        installLock(lock, index);

        installLock(lock, index, doWtf);

        return lock;

    }



    private static String lockToString(Object lock) {

        final LockInfo info = sKnown.get(lock);

        if (info != null) {

        if (info != null && !TextUtils.isEmpty(info.label)) {

            return info.label;

        } else {

            return "0x" + Integer.toHexString(System.identityHashCode(lock));
@@ -193,6 +230,7 @@ public class LockGuard { 
            case INDEX_STORAGE: return "STORAGE";

            case INDEX_WINDOW: return "WINDOW";

            case INDEX_ACTIVITY: return "ACTIVITY";

            case INDEX_DPMS: return "DPMS";

            default: return Integer.toString(index);

        }

    }

services/core/java/com/android/server/ThreadPriorityBooster.java

+3 −1
Original line number Diff line number Diff line
@@ -25,6 +25,8 @@ import static android.os.Process.setThreadPriority; 
 */

public class ThreadPriorityBooster {



    private static final boolean ENABLE_LOCK_GUARD = false;



    private volatile int mBoostToPriority;

    private final int mLockGuardIndex;
@@ -50,7 +52,7 @@ public class ThreadPriorityBooster { 
            }

        }

        state.regionCounter++;

        if (LockGuard.ENABLED) {

        if (ENABLE_LOCK_GUARD) {

            LockGuard.guard(mLockGuardIndex);

        }

    }

services/devicepolicy/java/com/android/server/devicepolicy/DevicePolicyManagerService.java

+402 −338

File changed.

Preview size limit exceeded, changes collapsed.