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

Commit f25a5183 authored by Suprabh Shukla's avatar Suprabh Shukla
Browse files

Use BinderDeathDispatcher for alarm listeners 

This ensures only one JNI strong reference for the death recipient when
the same binder is used for multiple linkToDeath calls.

Test: atest CtsAlarmManagerTestCases
atest FrameworksCoreTests:BinderDeathDispatcherTest
atest FrameworksMockingServicesTests:com.android.server.alarm

Fixes: 162961981
Bug: 182971683
Change-Id: Ieea2d8f4e495a4c4687378db9d0b8c6949a0406a
parent d6e2ea07

apex/jobscheduler/service/java/com/android/server/alarm/AlarmManagerService.java

+11 −3
Original line number Diff line number Diff line
@@ -118,6 +118,7 @@ import com.android.internal.annotations.GuardedBy; 
import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.app.IAppOpsCallback;

import com.android.internal.app.IAppOpsService;

import com.android.internal.os.BinderDeathDispatcher;

import com.android.internal.util.DumpUtils;

import com.android.internal.util.FrameworkStatsLog;

import com.android.internal.util.LocalLog;
@@ -202,6 +203,8 @@ public class AlarmManagerService extends SystemService { 
                    .addFlags(Intent.FLAG_RECEIVER_REPLACE_PENDING

                            | Intent.FLAG_RECEIVER_INCLUDE_BACKGROUND);



    private static final BinderDeathDispatcher<IAlarmListener> sListenerDeathDispatcher =

            new BinderDeathDispatcher<>();

    final LocalLog mLog = new LocalLog(TAG);



    AppOpsManager mAppOps;
@@ -1809,9 +1812,8 @@ public class AlarmManagerService extends SystemService { 
        }



        if (directReceiver != null) {

            try {

                directReceiver.asBinder().linkToDeath(mListenerDeathRecipient, 0);

            } catch (RemoteException e) {

            if (sListenerDeathDispatcher.linkToDeath(directReceiver, mListenerDeathRecipient)

                    <= 0) {

                Slog.w(TAG, "Dropping unreachable alarm listener " + listenerTag);

                return;

            }
@@ -2824,6 +2826,12 @@ public class AlarmManagerService extends SystemService { 
                pw.println();

            }



            pw.println("Listener death dispatcher state:");

            pw.increaseIndent();

            sListenerDeathDispatcher.dump(pw);

            pw.println();

            pw.decreaseIndent();



            if (mLog.dump(pw, "Recent problems:")) {

                pw.println();

            }

core/java/com/android/internal/os/BinderDeathDispatcher.java

+8 −6
Original line number Diff line number Diff line
@@ -24,12 +24,11 @@ import android.os.IInterface; 
import android.os.RemoteException;

import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.IndentingPrintWriter;



import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;



import java.io.PrintWriter;



/**

 * Multiplexes multiple binder death recipients on the same binder objects, so that at the native

 * level, we only need to keep track of one death recipient reference. This will help reduce the
@@ -63,6 +62,10 @@ public class BinderDeathDispatcher<T extends IInterface> { 


        @Override

        public void binderDied() {

        }



        @Override

        public void binderDied(IBinder who) {

            final ArraySet<DeathRecipient> copy;

            synchronized (mLock) {

                copy = mRecipients;
@@ -77,7 +80,7 @@ public class BinderDeathDispatcher<T extends IInterface> { 
            // Let's call it without holding the lock.

            final int size = copy.size();

            for (int i = 0; i < size; i++) {

                copy.valueAt(i).binderDied();

                copy.valueAt(i).binderDied(who);

            }

        }

    }
@@ -124,13 +127,12 @@ public class BinderDeathDispatcher<T extends IInterface> { 
        }

    }



    public void dump(PrintWriter pw, String indent) {

    /** Dump stats useful for debugging. Can be used from dump() methods of client services. */

    public void dump(IndentingPrintWriter pw) {

        synchronized (mLock) {

            pw.print(indent);

            pw.print("# of watched binders: ");

            pw.println(mTargets.size());



            pw.print(indent);

            pw.print("# of death recipients: ");

            int n = 0;

            for (RecipientsInfo info : mTargets.values()) {

core/java/com/android/internal/os/TEST_MAPPING

+10 −0
Original line number Diff line number Diff line
@@ -12,6 +12,16 @@ 
        { "exclude-annotation": "com.android.internal.os.SkipPresubmit" }

      ]

    },

    {

      "file_patterns": [

        "BinderDeathDispatcher\\.java"

      ],

      "name": "FrameworksCoreTests",

      "options": [

        { "include-filter": "com.android.internal.os.BinderDeathDispatcherTest" },

        { "exclude-annotation": "com.android.internal.os.SkipPresubmit" }

      ]

    },

    {

      "file_patterns": ["Battery[^/]*\\.java"],

      "name": "FrameworksServicesTests",

core/tests/coretests/src/com/android/internal/os/BinderDeathDispatcherTest.java

+43 −19
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ package com.android.internal.os; 


import static com.google.common.truth.Truth.assertThat;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.reset;

import static org.mockito.Mockito.times;
@@ -31,14 +32,14 @@ import android.os.RemoteException; 
import android.os.ResultReceiver;

import android.os.ShellCallback;



import androidx.test.filters.SmallTest;

import androidx.test.runner.AndroidJUnit4;



import org.junit.Test;

import org.junit.runner.RunWith;



import java.io.FileDescriptor;



import androidx.test.filters.SmallTest;

import androidx.test.runner.AndroidJUnit4;



@SmallTest

@RunWith(AndroidJUnit4.class)

public class BinderDeathDispatcherTest {
@@ -120,7 +121,7 @@ public class BinderDeathDispatcherTest { 
        public void die() {

            isAlive = false;

            if (mRecipient != null) {

                mRecipient.binderDied();

                mRecipient.binderDied(this);

            }

            mRecipient = null;

        }
@@ -227,33 +228,33 @@ public class BinderDeathDispatcherTest { 
        // Kill the targets.



        t1.die();

        verify(r1, times(1)).binderDied();

        verify(r2, times(1)).binderDied();

        verify(r3, times(1)).binderDied();

        verify(r4, times(0)).binderDied();

        verify(r5, times(0)).binderDied();

        verify(r1, times(1)).binderDied(t1);

        verify(r2, times(1)).binderDied(t1);

        verify(r3, times(1)).binderDied(t1);

        verify(r4, times(0)).binderDied(any());

        verify(r5, times(0)).binderDied(any());



        assertThat(d.getTargetsForTest().size()).isEqualTo(2);



        reset(r1, r2, r3, r4, r5);



        t2.die();

        verify(r1, times(1)).binderDied();

        verify(r2, times(0)).binderDied();

        verify(r3, times(0)).binderDied();

        verify(r4, times(0)).binderDied();

        verify(r5, times(0)).binderDied();

        verify(r1, times(1)).binderDied(t2);

        verify(r2, times(0)).binderDied(any());

        verify(r3, times(0)).binderDied(any());

        verify(r4, times(0)).binderDied(any());

        verify(r5, times(0)).binderDied(any());



        assertThat(d.getTargetsForTest().size()).isEqualTo(1);



        reset(r1, r2, r3, r4, r5);



        t3.die();

        verify(r1, times(0)).binderDied();

        verify(r2, times(0)).binderDied();

        verify(r3, times(1)).binderDied();

        verify(r4, times(0)).binderDied();

        verify(r5, times(1)).binderDied();

        verify(r1, times(0)).binderDied(any());

        verify(r2, times(0)).binderDied(any());

        verify(r3, times(1)).binderDied(t3);

        verify(r4, times(0)).binderDied(any());

        verify(r5, times(1)).binderDied(t3);



        assertThat(d.getTargetsForTest().size()).isEqualTo(0);
@@ -262,4 +263,27 @@ public class BinderDeathDispatcherTest { 


        assertThat(d.getTargetsForTest().size()).isEqualTo(0);

    }



    @Test

    public void duplicateRegistrations() {

        BinderDeathDispatcher<MyTarget> d = new BinderDeathDispatcher<>();



        MyTarget t1 = new MyTarget();



        DeathRecipient r1 = mock(DeathRecipient.class);

        DeathRecipient r2 = mock(DeathRecipient.class);



        for (int i = 0; i < 5; i++) {

            assertThat(d.linkToDeath(t1, r1)).isEqualTo(1);

        }

        assertThat(d.linkToDeath(t1, r2)).isEqualTo(2);



        t1.die();

        verify(r1, times(1)).binderDied(t1);

        verify(r2, times(1)).binderDied(t1);



        d.unlinkToDeath(t1, r1);

        d.unlinkToDeath(t1, r2);

        assertThat(d.getTargetsForTest()).isEmpty();

    }

}

services/core/java/com/android/server/content/ContentService.java

+5 −3
Original line number Diff line number Diff line
@@ -261,10 +261,12 @@ public final class ContentService extends IContentService.Stub { 
                    pw.print(pidCounts.get(pid)); pw.println(" observers");

                }

                pw.println();

                pw.increaseIndent();

                pw.print("Total number of nodes: "); pw.println(counts[0]);

                pw.print("Total number of observers: "); pw.println(counts[1]);



                sObserverDeathDispatcher.dump(pw, " ");

                sObserverDeathDispatcher.dump(pw);

                pw.decreaseIndent();

            }

            synchronized (sObserverLeakDetectedUid) {

                pw.println();