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

Commit 89d105d1 authored by Lee Shombert's avatar Lee Shombert
Browse files

Address AnrTimer review comments 

This CL address three review comments from an earlier commit:
 1. The call to register_android_server_utils_AnrTimer() is now
    sorted, per the style guide.
 2. An iterator error is fixed in AnrTimerService::Ticker::remove().
 3. NativeAllocationRegistry is used in lieu of finalizers in
    AnrTimer.

Ran AnrTimerTest.testGarbageCollection manually, in addition to the
automated tests.

Test: atest
 * FrameworksServicesTests:com.android.server.am
 * FrameworksMockingServicesTests:com.android.server.am
 * CtsAppTestCases
 * FrameworksServicesTests:AnrTimerTest

Bug: 282428924
Change-Id: I8cbbec2e74cd101685028958e450130678e4fdef
parent ab4ae770

services/core/java/com/android/server/utils/AnrTimer.java

+17 −14
Original line number Diff line number Diff line
@@ -36,6 +36,8 @@ import com.android.internal.annotations.Keep; 
import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.util.RingBuffer;



import libcore.util.NativeAllocationRegistry;



import java.lang.ref.WeakReference;

import java.io.PrintWriter;

import java.util.Arrays;
@@ -416,6 +418,14 @@ public class AnrTimer<V> implements AutoCloseable { 
    private static final LongSparseArray<WeakReference<AnrTimer>> sAnrTimerList =

        new LongSparseArray<>();



    /**

     * The manager of native pointers.

     */

    private static final NativeAllocationRegistry sNativeRegistry =

            NativeAllocationRegistry.createMalloced(

                AnrTimer.FeatureEnabled.class.getClassLoader(),

                nativeAnrTimerGetFinalizer());



    /**

     * The FeatureEnabled class enables the AnrTimer logic.  It is used when the AnrTimer service

     * is enabled via Flags.anrTimerServiceEnabled.
@@ -428,10 +438,14 @@ public class AnrTimer<V> implements AutoCloseable { 
         */

        private long mNative = 0;



        /** The Runnable that frees the native pointer. */

        private final Runnable mFreeNative;



        /** Fetch the native tag (an integer) for the given label. */

        FeatureEnabled() {

            mNative = nativeAnrTimerCreate(mLabel);

            if (mNative == 0) throw new IllegalArgumentException("unable to create native timer");

            mFreeNative = sNativeRegistry.registerNativeAllocation(this, mNative);

            synchronized (sAnrTimerList) {

                sAnrTimerList.put(mNative, new WeakReference(AnrTimer.this));

            }
@@ -542,7 +556,7 @@ public class AnrTimer<V> implements AutoCloseable { 
                sAnrTimerList.remove(mNative);

            }

            synchronized (mLock) {

                if (mNative != 0) nativeAnrTimerClose(mNative);

                if (mNative != 0) mFreeNative.run();

                mNative = 0;

            }

        }
@@ -658,17 +672,6 @@ public class AnrTimer<V> implements AutoCloseable { 
        mFeature.close();

    }



    /**

     * Ensure any native resources are freed when the object is GC'ed.  Best practice is to close

     * the object explicitly, but overriding finalize() avoids accidental leaks.

     */

    @SuppressWarnings("Finalize")

    @Override

    protected void finalize() throws Throwable {

        close();

        super.finalize();

    }



    /**

     * Dump a single AnrTimer.

     */
@@ -794,8 +797,8 @@ public class AnrTimer<V> implements AutoCloseable { 
     */

    private native long nativeAnrTimerCreate(String name);



    /** Release the native resources.  No further operations are premitted. */

    private static native int nativeAnrTimerClose(long service);

    /** Return the finalizer for the native resources. */

    private static native long nativeAnrTimerGetFinalizer();



    /** Start a timer and return its ID.  Zero is returned on error. */

    private static native int nativeAnrTimerStart(long service, int pid, int uid, long timeoutMs,

services/core/jni/com_android_server_utils_AnrTimer.cpp

+23 −10
Original line number Diff line number Diff line
@@ -486,9 +486,11 @@ class AnrTimerService::Ticker { 
    void remove(AnrTimerService const* service) {

        AutoMutex _l(lock_);

        timer_id_t front = headTimerId();

        for (auto i = running_.begin(); i != running_.end(); i++) {

        for (auto i = running_.begin(); i != running_.end(); ) {

            if (i->service == service) {

                running_.erase(i);

                i = running_.erase(i);

            } else {

                i++;

            }

        }

        if (front != headTimerId()) restartLocked();
@@ -808,7 +810,7 @@ static bool anrNotify(AnrTimerService::timer_id_t timerId, int pid, int uid, 
    AnrArgs* target = reinterpret_cast<AnrArgs* >(cookie);

    JNIEnv *env;

    if (target->vm->AttachCurrentThread(&env, 0) != JNI_OK) {

        ALOGE("failed to attach thread to JavaVM");

        ALOGE("anrNotify failed to attach thread to JavaVM");

        return false;

    }

    jboolean r = false;
@@ -844,18 +846,29 @@ AnrTimerService *toService(jlong pointer) { 
    return reinterpret_cast<AnrTimerService*>(pointer);

}



jint anrTimerClose(JNIEnv* env, jclass, jlong ptr) {

    if (!nativeSupportEnabled) return -1;

    if (ptr == 0) return -1;

AnrTimerService *toService(void* pointer) {

    return reinterpret_cast<AnrTimerService*>(pointer);

}



void anrTimerDelete(void *ptr) {

    if (ptr == 0) return;

    AutoMutex _l(gAnrLock);

    AnrTimerService* s = toService(ptr);

    JNIEnv *env;

    if (gAnrArgs.vm->AttachCurrentThread(&env, 0) == JNI_OK) {

        env->DeleteWeakGlobalRef(s->jtimer());

    } else {

        ALOGE("anrTimerDelete failed to attach thread to JavaVM");

    }

    delete s;

    if (--gAnrArgs.tickerUseCount <= 0) {

        delete gAnrArgs.ticker;

        gAnrArgs.ticker = nullptr;

    }

    return 0;

}



jlong anrTimerGetFinalizer(JNIEnv*, jclass) {

    return static_cast<jlong>(reinterpret_cast<uintptr_t>(&anrTimerDelete));

}



jint anrTimerStart(JNIEnv* env, jclass, jlong ptr,
@@ -890,7 +903,7 @@ jint anrTimerDump(JNIEnv *env, jclass, jlong ptr, jboolean verbose) { 
static const JNINativeMethod methods[] = {

    {"nativeAnrTimerSupported", "()Z",  (void*) anrTimerSupported},

    {"nativeAnrTimerCreate", "(Ljava/lang/String;)J", (void*) anrTimerCreate},

    {"nativeAnrTimerClose", "(J)I",     (void*) anrTimerClose},

    {"nativeAnrTimerGetFinalizer", "()J", (void*) anrTimerGetFinalizer},

    {"nativeAnrTimerStart", "(JIIJZ)I", (void*) anrTimerStart},

    {"nativeAnrTimerCancel", "(JI)Z",   (void*) anrTimerCancel},

    {"nativeAnrTimerAccept", "(JI)Z",   (void*) anrTimerAccept},

services/core/jni/onload.cpp

+2 −2
Original line number Diff line number Diff line
@@ -52,10 +52,10 @@ int register_android_server_Watchdog(JNIEnv* env); 
int register_android_server_HardwarePropertiesManagerService(JNIEnv* env);

int register_android_server_SyntheticPasswordManager(JNIEnv* env);

int register_android_hardware_display_DisplayViewport(JNIEnv* env);

int register_android_server_utils_AnrTimer(JNIEnv *env);

int register_android_server_am_OomConnection(JNIEnv* env);

int register_android_server_am_CachedAppOptimizer(JNIEnv* env);

int register_android_server_am_LowMemDetector(JNIEnv* env);

int register_android_server_utils_AnrTimer(JNIEnv *env);

int register_com_android_server_soundtrigger_middleware_AudioSessionProviderImpl(JNIEnv* env);

int register_com_android_server_soundtrigger_middleware_ExternalCaptureStateTracker(JNIEnv* env);

int register_android_server_com_android_server_pm_PackageManagerShellCommandDataLoader(JNIEnv* env);
@@ -114,10 +114,10 @@ extern "C" jint JNI_OnLoad(JavaVM* vm, void* /* reserved */) 
    register_android_server_storage_AppFuse(env);

    register_android_server_SyntheticPasswordManager(env);

    register_android_hardware_display_DisplayViewport(env);

    register_android_server_utils_AnrTimer(env);

    register_android_server_am_OomConnection(env);

    register_android_server_am_CachedAppOptimizer(env);

    register_android_server_am_LowMemDetector(env);

    register_android_server_utils_AnrTimer(env);

    register_com_android_server_soundtrigger_middleware_AudioSessionProviderImpl(env);

    register_com_android_server_soundtrigger_middleware_ExternalCaptureStateTracker(env);

    register_android_server_com_android_server_pm_PackageManagerShellCommandDataLoader(env);