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

4 files changed, 67 insertions, 23 deletions

diff --git a/core/java/com/android/internal/os/BinderDeathDispatcher.java b/core/java/com/android/internal/os/BinderDeathDispatcher.java
index 0c93f7f160e4..038707ee1a4c 100644
--- a/core/java/com/android/internal/os/BinderDeathDispatcher.java
+++ b/core/java/com/android/internal/os/BinderDeathDispatcher.java
@@ -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);
             }
         }
     }
diff --git a/core/java/com/android/internal/os/TEST_MAPPING b/core/java/com/android/internal/os/TEST_MAPPING
index f44b9fb7e723..fdc3a9ee3dec 100644
--- a/core/java/com/android/internal/os/TEST_MAPPING
+++ b/core/java/com/android/internal/os/TEST_MAPPING
@@ -1,6 +1,16 @@
 {
   "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": [
         {
diff --git a/core/tests/coretests/src/com/android/internal/os/BinderDeathDispatcherTest.java b/core/tests/coretests/src/com/android/internal/os/BinderDeathDispatcherTest.java
index 942045c8bf35..83103333f68b 100644
--- a/core/tests/coretests/src/com/android/internal/os/BinderDeathDispatcherTest.java
+++ b/core/tests/coretests/src/com/android/internal/os/BinderDeathDispatcherTest.java
@@ -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();
+    }
 }
diff --git a/services/core/java/com/android/server/AlarmManagerService.java b/services/core/java/com/android/server/AlarmManagerService.java
index 7cdcc01bc00d..781c3540d8aa 100644
--- a/services/core/java/com/android/server/AlarmManagerService.java
+++ b/services/core/java/com/android/server/AlarmManagerService.java
@@ -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();
             }