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

Commit 715ecc2b 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:AlarmManagerServiceTest

Bug: 186792832
Change-Id: Ieea2d8f4e495a4c4687378db9d0b8c6949a0406a
Merged-In: Ieea2d8f4e495a4c4687378db9d0b8c6949a0406a
parent 4e8328c9

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

+5 −1
Original line number Diff line number Diff line
@@ -63,6 +63,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 +81,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);

            }

        }

    }

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

+10 −0
Original line number Diff line number Diff line 
{

  "presubmit": [

    {

      "file_patterns": [

        "BinderDeathDispatcher\\.java"

      ],

      "name": "FrameworksCoreTests",

      "options": [

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

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

      ]

    },

    {

      "name": "FrameworksCoreTests",

      "options": [

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/AlarmManagerService.java

+9 −3
Original line number Diff line number Diff line
@@ -91,6 +91,7 @@ import android.util.proto.ProtoOutputStream; 


import com.android.internal.annotations.GuardedBy;

import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.os.BinderDeathDispatcher;

import com.android.internal.util.ArrayUtils;

import com.android.internal.util.DumpUtils;

import com.android.internal.util.FrameworkStatsLog;
@@ -176,6 +177,8 @@ 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;
@@ -1701,9 +1704,8 @@ 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;

            }
@@ -2464,6 +2466,10 @@ class AlarmManagerService extends SystemService { 
            pw.println("]");

            pw.println();



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

            sListenerDeathDispatcher.dump(pw, "  ");

            pw.println();



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

                pw.println();

            }