From 345a42452b5c10f05fb5fb2d1753712ac030a731 Mon Sep 17 00:00:00 2001 From: Austin Borger Date: Tue, 6 Jun 2023 12:46:07 -0700 Subject: [PATCH] Run addUidToObserver and removeUidFromObserver on the handler thread. If we're in dispatchUidsChanged while another thread is calling addUidToObserver or removeUidFromObserver, there will be two active broadcasts for mUidObservers. Move this processing to the handler thread to eliminate the possibility of this race condition. Change-Id: I9d64978415ebb42f194936ec56ba9933ed1533e7 Bug: 285982408 Test: Ran testCamera2AccessCallbackInSplitMode --- .../server/am/ActivityManagerService.java | 8 ++ .../server/am/UidObserverController.java | 83 ++++++++++++------- 2 files changed, 61 insertions(+), 30 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 92e73f585466e..4decbd19c762d 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -1616,6 +1616,8 @@ public class ActivityManagerService extends IActivityManager.Stub static final int SERVICE_SHORT_FGS_PROCSTATE_TIMEOUT_MSG = 77; static final int SERVICE_SHORT_FGS_ANR_TIMEOUT_MSG = 78; static final int UPDATE_CACHED_APP_HIGH_WATERMARK = 79; + static final int ADD_UID_TO_OBSERVER_MSG = 80; + static final int REMOVE_UID_FROM_OBSERVER_MSG = 81; static final int FIRST_BROADCAST_QUEUE_MSG = 200; @@ -1774,6 +1776,12 @@ public class ActivityManagerService extends IActivityManager.Stub case PUSH_TEMP_ALLOWLIST_UI_MSG: { pushTempAllowlist(); } break; + case ADD_UID_TO_OBSERVER_MSG: { + mUidObserverController.addUidToObserverImpl((IBinder) msg.obj, msg.arg1); + } break; + case REMOVE_UID_FROM_OBSERVER_MSG: { + mUidObserverController.removeUidFromObserverImpl((IBinder) msg.obj, msg.arg1); + } break; } } } diff --git a/services/core/java/com/android/server/am/UidObserverController.java b/services/core/java/com/android/server/am/UidObserverController.java index a6677a5185caa..7eeec32cf24a8 100644 --- a/services/core/java/com/android/server/am/UidObserverController.java +++ b/services/core/java/com/android/server/am/UidObserverController.java @@ -30,6 +30,7 @@ import android.content.pm.PackageManager; import android.os.Binder; import android.os.Handler; import android.os.IBinder; +import android.os.Message; import android.os.RemoteCallbackList; import android.os.RemoteException; import android.os.SystemClock; @@ -104,40 +105,62 @@ public class UidObserverController { } } - void addUidToObserver(@NonNull IBinder observerToken, int uid) { - synchronized (mLock) { - int i = mUidObservers.beginBroadcast(); - while (i-- > 0) { - var reg = (UidObserverRegistration) mUidObservers.getBroadcastCookie(i); - if (reg.getToken().equals(observerToken)) { - reg.addUid(uid); - break; - } - - if (i == 0) { - Slog.e(TAG_UID_OBSERVERS, "Unable to find UidObserver by token"); - } - } - mUidObservers.finishBroadcast(); - } + final void addUidToObserver(@NonNull IBinder observerToken, int uid) { + Message msg = Message.obtain(mHandler, ActivityManagerService.ADD_UID_TO_OBSERVER_MSG, + uid, /*arg2*/ 0, observerToken); + mHandler.sendMessage(msg); } - void removeUidFromObserver(@NonNull IBinder observerToken, int uid) { - synchronized (mLock) { - int i = mUidObservers.beginBroadcast(); - while (i-- > 0) { - var reg = (UidObserverRegistration) mUidObservers.getBroadcastCookie(i); - if (reg.getToken().equals(observerToken)) { - reg.removeUid(uid); - break; - } - - if (i == 0) { - Slog.e(TAG_UID_OBSERVERS, "Unable to find UidObserver by token"); - } + /** + * Add a uid to the list of uids an observer is interested in. Must be run on the same thread + * as mDispatchRunnable. + * + * @param observerToken The token identifier for a UidObserver + * @param uid The uid to add to the list of watched uids + */ + public final void addUidToObserverImpl(@NonNull IBinder observerToken, int uid) { + int i = mUidObservers.beginBroadcast(); + while (i-- > 0) { + var reg = (UidObserverRegistration) mUidObservers.getBroadcastCookie(i); + if (reg.getToken().equals(observerToken)) { + reg.addUid(uid); + break; + } + + if (i == 0) { + Slog.e(TAG_UID_OBSERVERS, "Unable to find UidObserver by token"); } - mUidObservers.finishBroadcast(); } + mUidObservers.finishBroadcast(); + } + + final void removeUidFromObserver(@NonNull IBinder observerToken, int uid) { + Message msg = Message.obtain(mHandler, ActivityManagerService.REMOVE_UID_FROM_OBSERVER_MSG, + uid, /*arg2*/ 0, observerToken); + mHandler.sendMessage(msg); + } + + /** + * Remove a uid from the list of uids an observer is interested in. Must be run on the same + * thread as mDispatchRunnable. + * + * @param observerToken The token identifier for a UidObserver + * @param uid The uid to remove from the list of watched uids + */ + public final void removeUidFromObserverImpl(@NonNull IBinder observerToken, int uid) { + int i = mUidObservers.beginBroadcast(); + while (i-- > 0) { + var reg = (UidObserverRegistration) mUidObservers.getBroadcastCookie(i); + if (reg.getToken().equals(observerToken)) { + reg.removeUid(uid); + break; + } + + if (i == 0) { + Slog.e(TAG_UID_OBSERVERS, "Unable to find UidObserver by token"); + } + } + mUidObservers.finishBroadcast(); } int enqueueUidChange(@Nullable ChangeRecord currentRecord, int uid, int change, int procState,