Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Avoid creating multiple death recipients for same observer" into qt-dev

Browse Source 
am: a478c5f9aa

Change-Id: I66242d7ce6acdd9850965328ffa79e9b1a2e8f4d
This commit is contained in:
Makoto Onuki
2019-06-05 09:08:37 -07:00
committed by android-build-merger
parent f550948c4c a478c5f9aa
commit f574979713
5 changed files with 464 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

147
core/java/com/android/internal/os/BinderDeathDispatcher.java Normal file
View File

@@ -0,0 +1,147 @@
/*
* Copyright (C) 2019 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.internal.os;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.os.IBinder;
import android.os.IBinder.DeathRecipient;
import android.os.IInterface;
import android.os.RemoteException;
import android.util.ArrayMap;
import android.util.ArraySet;
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
* number of JNI strong references.
*
* test with: atest FrameworksCoreTests:BinderDeathDispatcherTest
*/
public class BinderDeathDispatcher<T extends IInterface> {
private static final String TAG = "BinderDeathDispatcher";
private final Object mLock = new Object();
@GuardedBy("mLock")
private final ArrayMap<IBinder, RecipientsInfo> mTargets = new ArrayMap<>();
@VisibleForTesting
class RecipientsInfo implements DeathRecipient {
final IBinder mTarget;
/**
* Recipient list. If it's null, {@link #mTarget} has already died, but in that case
* this RecipientsInfo instance is removed from {@link #mTargets}.
*/
@GuardedBy("mLock")
@Nullable
ArraySet<DeathRecipient> mRecipients = new ArraySet<>();
private RecipientsInfo(IBinder target) {
mTarget = target;
}
@Override
public void binderDied() {
final ArraySet<DeathRecipient> copy;
synchronized (mLock) {
copy = mRecipients;
mRecipients = null;
// Also remove from the targets.
mTargets.remove(mTarget);
}
if (copy == null) {
return;
}
// Let's call it without holding the lock.
final int size = copy.size();
for (int i = 0; i < size; i++) {
copy.valueAt(i).binderDied();
}
}
}
/**
* Add a {@code recipient} to the death recipient list on {@code target}.
*
* @return # of recipients in the recipient list, including {@code recipient}. Or, -1
* if {@code target} is already dead, in which case recipient's
* {@link DeathRecipient#binderDied} won't be called.
*/
public int linkToDeath(@NonNull T target, @NonNull DeathRecipient recipient) {
final IBinder ib = target.asBinder();
synchronized (mLock) {
RecipientsInfo info = mTargets.get(ib);
if (info == null) {
info = new RecipientsInfo(ib);
// First recipient; need to link to death.
try {
ib.linkToDeath(info, 0);
} catch (RemoteException e) {
return -1; // Already dead.
}
mTargets.put(ib, info);
}
info.mRecipients.add(recipient);
return info.mRecipients.size();
}
}
public void unlinkToDeath(@NonNull T target, @NonNull DeathRecipient recipient) {
final IBinder ib = target.asBinder();
synchronized (mLock) {
final RecipientsInfo info = mTargets.get(ib);
if (info == null) {
return;
}
if (info.mRecipients.remove(recipient) && info.mRecipients.size() == 0) {
info.mTarget.unlinkToDeath(info, 0);
mTargets.remove(info.mTarget);
}
}
}
public void dump(PrintWriter pw, String indent) {
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()) {
n += info.mRecipients.size();
}
pw.println(n);
}
}
@VisibleForTesting
public ArrayMap<IBinder, RecipientsInfo> getTargetsForTest() {
return mTargets;
}
}

4
core/java/com/android/internal/util/ArrayUtils.java
View File

@@ -756,4 +756,8 @@ public class ArrayUtils {
return String.valueOf(value);
}
}
public static @Nullable <T> T firstOrNull(T[] items) {
return items.length > 0 ? items[0] : null;
}
}

265
core/tests/coretests/src/com/android/internal/os/BinderDeathDispatcherTest.java Normal file
View File

@@ -0,0 +1,265 @@
/*
* Copyright (C) 2019 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.android.internal.os;
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import android.os.DeadObjectException;
import android.os.IBinder;
import android.os.IBinder.DeathRecipient;
import android.os.IInterface;
import android.os.Parcel;
import android.os.RemoteException;
import android.os.ResultReceiver;
import android.os.ShellCallback;
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 {
private static class MyTarget implements IInterface, IBinder {
public boolean isAlive = true;
public DeathRecipient mRecipient;
@Override
public String getInterfaceDescriptor() throws RemoteException {
return null;
}
@Override
public boolean pingBinder() {
return false;
}
@Override
public boolean isBinderAlive() {
return isAlive;
}
@Override
public IInterface queryLocalInterface(String descriptor) {
return null;
}
@Override
public void dump(FileDescriptor fd, String[] args) throws RemoteException {
}
@Override
public void dumpAsync(FileDescriptor fd, String[] args) throws RemoteException {
}
@Override
public void shellCommand(FileDescriptor in, FileDescriptor out, FileDescriptor err,
String[] args, ShellCallback shellCallback, ResultReceiver resultReceiver)
throws RemoteException {
}
@Override
public boolean transact(int code, Parcel data, Parcel reply, int flags)
throws RemoteException {
return false;
}
@Override
public void linkToDeath(DeathRecipient recipient, int flags) throws RemoteException {
// In any situation, a single binder object should only have at most one death
// recipient.
assertThat(mRecipient).isNull();
if (!isAlive) {
throw new DeadObjectException();
}
mRecipient = recipient;
}
@Override
public boolean unlinkToDeath(DeathRecipient recipient, int flags) {
if (!isAlive) {
return false;
}
assertThat(mRecipient).isSameAs(recipient);
mRecipient = null;
return true;
}
@Override
public IBinder asBinder() {
return this;
}
public void die() {
isAlive = false;
if (mRecipient != null) {
mRecipient.binderDied();
}
mRecipient = null;
}
public boolean hasDeathRecipient() {
return mRecipient != null;
}
}
@Test
public void testRegisterAndUnregister() {
BinderDeathDispatcher<MyTarget> d = new BinderDeathDispatcher<>();
MyTarget t1 = new MyTarget();
MyTarget t2 = new MyTarget();
MyTarget t3 = new MyTarget();
DeathRecipient r1 = mock(DeathRecipient.class);
DeathRecipient r2 = mock(DeathRecipient.class);
DeathRecipient r3 = mock(DeathRecipient.class);
DeathRecipient r4 = mock(DeathRecipient.class);
DeathRecipient r5 = mock(DeathRecipient.class);
// Start hooking up.
// Link 3 recipients to t1 -- only one real recipient will be set.
assertThat(d.linkToDeath(t1, r1)).isEqualTo(1);
assertThat(d.getTargetsForTest().size()).isEqualTo(1);
assertThat(d.linkToDeath(t1, r2)).isEqualTo(2);
assertThat(d.linkToDeath(t1, r3)).isEqualTo(3);
assertThat(d.getTargetsForTest().size()).isEqualTo(1);
// Unlink two -- the real recipient is still set.
d.unlinkToDeath(t1, r1);
d.unlinkToDeath(t1, r2);
assertThat(t1.hasDeathRecipient()).isTrue();
assertThat(d.getTargetsForTest().size()).isEqualTo(1);
// Unlink the last one. The real recipient is also unlinked.
d.unlinkToDeath(t1, r3);
assertThat(t1.hasDeathRecipient()).isFalse();
assertThat(d.getTargetsForTest().size()).isEqualTo(0);
// Set recipients to t1, t2 and t3. t3 has two.
assertThat(d.linkToDeath(t1, r1)).isEqualTo(1);
assertThat(d.linkToDeath(t2, r1)).isEqualTo(1);
assertThat(d.linkToDeath(t3, r1)).isEqualTo(1);
assertThat(d.linkToDeath(t3, r2)).isEqualTo(2);
// They should all have a real recipient.
assertThat(t1.hasDeathRecipient()).isTrue();
assertThat(t2.hasDeathRecipient()).isTrue();
assertThat(t3.hasDeathRecipient()).isTrue();
assertThat(d.getTargetsForTest().size()).isEqualTo(3);
// Unlink r1 from t3. t3 still has r2, so it should still have a real recipient.
d.unlinkToDeath(t3, r1);
assertThat(t1.hasDeathRecipient()).isTrue();
assertThat(t2.hasDeathRecipient()).isTrue();
assertThat(t3.hasDeathRecipient()).isTrue();
assertThat(d.getTargetsForTest().size()).isEqualTo(3);
// Unlink r2 from t3. Now t3 has no real recipient.
d.unlinkToDeath(t3, r2);
assertThat(t3.hasDeathRecipient()).isFalse();
assertThat(d.getTargetsForTest().size()).isEqualTo(2);
}
@Test
public void testRegisterAndKill() {
BinderDeathDispatcher<MyTarget> d = new BinderDeathDispatcher<>();
MyTarget t1 = new MyTarget();
MyTarget t2 = new MyTarget();
MyTarget t3 = new MyTarget();
DeathRecipient r1 = mock(DeathRecipient.class);
DeathRecipient r2 = mock(DeathRecipient.class);
DeathRecipient r3 = mock(DeathRecipient.class);
DeathRecipient r4 = mock(DeathRecipient.class);
DeathRecipient r5 = mock(DeathRecipient.class);
// Hook them up.
d.linkToDeath(t1, r1);
d.linkToDeath(t1, r2);
d.linkToDeath(t1, r3);
// r4 is linked then unlinked. It shouldn't be notified.
d.linkToDeath(t1, r4);
d.unlinkToDeath(t1, r4);
d.linkToDeath(t2, r1);
d.linkToDeath(t3, r3);
d.linkToDeath(t3, r5);
assertThat(d.getTargetsForTest().size()).isEqualTo(3);
// 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();
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();
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();
assertThat(d.getTargetsForTest().size()).isEqualTo(0);
// Try to register to a dead object -> should return -1.
assertThat(d.linkToDeath(t1, r1)).isEqualTo(-1);
assertThat(d.getTargetsForTest().size()).isEqualTo(0);
}
}

53
services/core/java/com/android/server/content/ContentService.java
View File

@@ -22,6 +22,7 @@ import android.annotation.Nullable;
import android.annotation.RequiresPermission;
import android.app.ActivityManager;
import android.app.ActivityManagerInternal;
import android.app.AppGlobals;
import android.app.AppOpsManager;
import android.app.job.JobInfo;
import android.content.BroadcastReceiver;
@@ -56,6 +57,7 @@ import android.os.ShellCallback;
import android.os.UserHandle;
import android.text.TextUtils;
import android.util.ArrayMap;
import android.util.ArraySet;
import android.util.Log;
import android.util.Pair;
import android.util.Slog;
@@ -63,6 +65,7 @@ import android.util.SparseArray;
import android.util.SparseIntArray;
import com.android.internal.annotations.GuardedBy;
import com.android.internal.os.BinderDeathDispatcher;
import com.android.internal.util.ArrayUtils;
import com.android.internal.util.DumpUtils;
import com.android.internal.util.IndentingPrintWriter;
@@ -83,6 +86,9 @@ public final class ContentService extends IContentService.Stub {
static final String TAG = "ContentService";
static final boolean DEBUG = false;
/** Do a WTF if a single observer is registered more than this times. */
private static final int TOO_MANY_OBSERVERS_THRESHOLD = 1000;
public static class Lifecycle extends SystemService {
private ContentService mService;
@@ -135,6 +141,12 @@ public final class ContentService extends IContentService.Stub {
private SyncManager mSyncManager = null;
private final Object mSyncManagerLock = new Object();
private static final BinderDeathDispatcher<IContentObserver> sObserverDeathDispatcher =
new BinderDeathDispatcher<>();
@GuardedBy("sObserverLeakDetectedUid")
private static final ArraySet<Integer> sObserverLeakDetectedUid = new ArraySet<>(0);
/**
* Map from userId to providerPackageName to [clientPackageName, uri] to
* value. This structure is carefully optimized to keep invalidation logic
@@ -236,6 +248,13 @@ public final class ContentService extends IContentService.Stub {
pw.println();
pw.print(" Total number of nodes: "); pw.println(counts[0]);
pw.print(" Total number of observers: "); pw.println(counts[1]);
sObserverDeathDispatcher.dump(pw, " ");
}
synchronized (sObserverLeakDetectedUid) {
pw.println();
pw.print("Observer leaking UIDs: ");
pw.println(sObserverLeakDetectedUid.toString());
}
synchronized (mCache) {
@@ -1345,18 +1364,40 @@ public final class ContentService extends IContentService.Stub {
private final Object observersLock;
public ObserverEntry(IContentObserver o, boolean n, Object observersLock,
int _uid, int _pid, int _userHandle) {
int _uid, int _pid, int _userHandle, Uri uri) {
this.observersLock = observersLock;
observer = o;
uid = _uid;
pid = _pid;
userHandle = _userHandle;
notifyForDescendants = n;
try {
observer.asBinder().linkToDeath(this, 0);
} catch (RemoteException e) {
final int entries = sObserverDeathDispatcher.linkToDeath(observer, this);
if (entries == -1) {
binderDied();
} else if (entries == TOO_MANY_OBSERVERS_THRESHOLD) {
boolean alreadyDetected;
synchronized (sObserverLeakDetectedUid) {
alreadyDetected = sObserverLeakDetectedUid.contains(uid);
if (!alreadyDetected) {
sObserverLeakDetectedUid.add(uid);
}
}
if (!alreadyDetected) {
String caller = null;
try {
caller = ArrayUtils.firstOrNull(AppGlobals.getPackageManager()
.getPackagesForUid(uid));
} catch (RemoteException ignore) {
}
Slog.wtf(TAG, "Observer registered too many times. Leak? cpid=" + pid
+ " cuid=" + uid
+ " cpkg=" + caller
+ " url=" + uri);
}
}
}
@Override
@@ -1454,7 +1495,7 @@ public final class ContentService extends IContentService.Stub {
// If this is the leaf node add the observer
if (index == countUriSegments(uri)) {
mObservers.add(new ObserverEntry(observer, notifyForDescendants, observersLock,
uid, pid, userHandle));
uid, pid, userHandle, uri));
return;
}
@@ -1498,7 +1539,7 @@ public final class ContentService extends IContentService.Stub {
if (entry.observer.asBinder() == observerBinder) {
mObservers.remove(i);
// We no longer need to listen for death notifications. Remove it.
observerBinder.unlinkToDeath(entry, 0);
sObserverDeathDispatcher.unlinkToDeath(observer, entry);
break;
}
}

2
services/tests/servicestests/src/com/android/server/content/ObserverNodeTest.java
View File

@@ -30,7 +30,7 @@ import com.android.server.content.ContentService.ObserverCall;
import com.android.server.content.ContentService.ObserverNode;
/**
* bit FrameworksServicesTests:com.android.server.content.ObserverNodeTest
* atest FrameworksServicesTests:com.android.server.content.ObserverNodeTest
*/
@SmallTest
public class ObserverNodeTest extends AndroidTestCase {